Iago Toral <ito...@igalia.com> writes:
> 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.
>
Heh, I did that mainly out of laziness because it was less noise than
declaring a new variable. It shouldn't make any practical difference
-- Or was your suggestion on purely stylistic grounds?
>> + 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... ;)
>
Sure, I'll throw in some more comments.
> The implementation looks correct to me:
> Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
>
Thanks.
>> + 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;
>> }
>>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
