[Mesa-dev] [PATCH 78/78] nir: Fix output swizzle in get_mul_for_src

[Samuel Iglesias Gonsálvez]

On 01/07/15 01:41, Jason Ekstrand wrote:
> On Fri, Jun 26, 2015 at 1:07 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>
>> Avoid copying an overwritten swizzle, use the original values.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>> ---
>>  src/glsl/nir/nir_opt_peephole_ffma.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/nir/nir_opt_peephole_ffma.c b/src/glsl/nir/nir_opt_peephole_ffma.c
>> index 798506b..7c9ac12 100644
>> --- a/src/glsl/nir/nir_opt_peephole_ffma.c
>> +++ b/src/glsl/nir/nir_opt_peephole_ffma.c
>> @@ -76,6 +76,7 @@ static nir_alu_instr *
>>  get_mul_for_src(nir_alu_src *src, int num_components,
>>                  uint8_t swizzle[4], bool *negate, bool *abs)
>>  {
>> +   uint8_t swizzle_tmp[4];
>>     assert(src->src.is_ssa && !src->abs && !src->negate);
>>
>>     nir_instr *instr = src->src.ssa->parent_instr;
>> @@ -116,8 +117,10 @@ get_mul_for_src(nir_alu_src *src, int num_components,
>>     if (!alu)
>>        return NULL;
>>
>> +   /* Copy swizzle data before overwriting it */
> 
> I'd like the comment to be a little more detailed.  Specifically, an
> example of when this is a problem would be really good.  Looking at
> it, I *think* that if src->swizzle is xyxx, it would trip the problem.
> 

I found the issue in a test that does something similar to the following:

src->swizzle = zyxx
swizzle = xyzw

As we are overwritting swizzle, we end up having:

final swizzle = zyzz

But the expected output swizzle is zyxx.

I am going to rewrite the comment adding this example.

Thanks for the review,

Sam

>> +   memcpy(swizzle_tmp, swizzle, 4*sizeof(uint8_t));
>>     for (unsigned i = 0; i < num_components; i++)
>> -      swizzle[i] = swizzle[src->swizzle[i]];
>> +      swizzle[i] = swizzle_tmp[src->swizzle[i]];
>>
>>     return alu;
>>  }
>> --
>> 2.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>