On Fri, 2016-02-26 at 22:02 -0800, Francisco Jerez wrote:
> Scalar immediates used to be handled correctly by swizzle() (as the
> identity) but since commit 58fa9d47b536403c4e3ca5d6a2495691338388fd it
> will corrupt the contents of the immediate. Vector immediates were
> never handled correctly, but we had ad-hoc code to swizzle VF
> immediates in the vec4 copy propagation pass. This takes care of
> swizzling V and UV in addition.
> ---
> src/mesa/drivers/dri/i965/brw_eu.c | 39
> +++++++++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_ir_vec4.h | 6 ++++-
> src/mesa/drivers/dri/i965/brw_reg.h | 7 ++++--
> 3 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.c
> b/src/mesa/drivers/dri/i965/brw_eu.c
> index 40ec87d..36bdc7c 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu.c
> @@ -110,6 +110,45 @@ brw_swap_cmod(uint32_t cmod)
> }
> }
>
> +/**
> + * Get the least significant bit offset of the i+1-th component of immediate
> + * type \p type. For \p i equal to the two's complement of j, return the
> + * offset of the j-th component starting from the end of the vector. For
> + * scalar register types return zero.
> + */
> +static unsigned
> +imm_shift(enum brw_reg_type type, unsigned i)
> +{
> + if (type == BRW_REGISTER_TYPE_VF)
> + return 8 * (i & 3);
> + else if (type == BRW_REGISTER_TYPE_UV || type == BRW_REGISTER_TYPE_V)
> + return 4 * (i & 7);
> + else
> + return 0;
> +}
> +
> +/**
> + * Swizzle an arbitrary immediate \p x of the given type according to the
> + * permutation specified as \p swz.
> + */
> +uint32_t
> +brw_swizzle_immediate(enum brw_reg_type type, uint32_t x, unsigned swz)
> +{
> + if (imm_shift(type, 1)) {
> + const unsigned n = 32 / imm_shift(type, 1);
You can assign imm_shift(type, 1) to a variable before the if and reuse
it here.
> + uint32_t y = 0;
> +
> + for (unsigned i = 0; i < n; i++)
> + y |= x >> imm_shift(type, (i & ~3) + BRW_GET_SWZ(swz, i & 3))
> + << imm_shift(type, ~0u)
> + >> imm_shift(type, ~0u - i);
> +
Ugh, took me a while to understand what this was doing even with the
comment in imm_shift(). Another comment here explaining what this series
of operations are doing might save future readers some time... ;)
The implementation looks correct to me:
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
> + return y;
> + } else {
> + return x;
> + }
> +}
> +
> void
> brw_set_default_exec_size(struct brw_codegen *p, unsigned value)
> {
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> index 660beca..2b6872e 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> @@ -76,7 +76,11 @@ offset(src_reg reg, unsigned delta)
> static inline src_reg
> swizzle(src_reg reg, unsigned swizzle)
> {
> - reg.swizzle = brw_compose_swizzle(swizzle, reg.swizzle);
> + if (reg.file == IMM)
> + reg.ud = brw_swizzle_immediate(reg.type, reg.ud, swizzle);
> + else
> + reg.swizzle = brw_compose_swizzle(swizzle, reg.swizzle);
> +
> return reg;
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h
> b/src/mesa/drivers/dri/i965/brw_reg.h
> index a4bcfca..74ff67f 100644
> --- a/src/mesa/drivers/dri/i965/brw_reg.h
> +++ b/src/mesa/drivers/dri/i965/brw_reg.h
> @@ -223,6 +223,7 @@ enum PACKED brw_reg_type {
> unsigned brw_reg_type_to_hw_type(const struct brw_device_info *devinfo,
> enum brw_reg_type type, enum brw_reg_file
> file);
> const char *brw_reg_type_letters(unsigned brw_reg_type);
> +uint32_t brw_swizzle_immediate(enum brw_reg_type type, uint32_t x, unsigned
> swz);
>
> #define REG_SIZE (8*4)
>
> @@ -876,9 +877,11 @@ get_element_d(struct brw_reg reg, unsigned elt)
> static inline struct brw_reg
> brw_swizzle(struct brw_reg reg, unsigned swz)
> {
> - assert(reg.file != BRW_IMMEDIATE_VALUE);
> + if (reg.file == BRW_IMMEDIATE_VALUE)
> + reg.ud = brw_swizzle_immediate(reg.type, reg.ud, swz);
> + else
> + reg.swizzle = brw_compose_swizzle(swz, reg.swizzle);
>
> - reg.swizzle = brw_compose_swizzle(swz, reg.swizzle);
> return reg;
> }
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
