[Mesa-dev] [PATCH] nir/cse: fix bug with comparing non-per-component sources

[Connor Abbott]

On Tue, Apr 14, 2015 at 4:56 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Tue, Apr 14, 2015 at 1:39 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>> On Tue, Apr 14, 2015 at 3:55 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>>> On Tue, Apr 14, 2015 at 12:51 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>>> We weren't comparing the right number of components when checking
>>>> swizzles. Use nir_ssa_alu_instr_num_src_components() to do the right
>>>
>>> I don't like the name of that function, but oh well.
>>
>> Suggestions for a better name are welcome :).
>
> How about nir_alu_instr_ssa_src_num_components or
> nir_alu_instr_num_ssa_src_components?
> --Jason

I think I had something like that initially, but you wanted to change
it since it actually depends on the destination of the instr being
SSA. Both of those, especially the second one, seem to imply that the
src is SSA (and not necessarily the dest).

Connor

>
>>>
>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>
>>>> thing.
>>>>
>>>> I haven't piglited this yet, but it seems straightforward.
>>>>
>>>> Cc: Ian Romanick <ian.d.romanick at intel.com>
>>>> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
>>>> ---
>>>>  src/glsl/nir/nir_opt_cse.c | 17 +++++++----------
>>>>  1 file changed, 7 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/src/glsl/nir/nir_opt_cse.c b/src/glsl/nir/nir_opt_cse.c
>>>> index 9b38320..db873ce 100644
>>>> --- a/src/glsl/nir/nir_opt_cse.c
>>>> +++ b/src/glsl/nir/nir_opt_cse.c
>>>> @@ -37,20 +37,18 @@ struct cse_state {
>>>>  };
>>>>
>>>>  static bool
>>>> -nir_alu_srcs_equal(nir_alu_src src1, nir_alu_src src2, uint8_t read_mask)
>>>> +nir_alu_srcs_equal(nir_alu_instr *alu1, nir_alu_instr *alu2, unsigned src)
>>>>  {
>>>> -   if (src1.abs != src2.abs || src1.negate != src2.negate)
>>>> +   if (alu1->src[src].abs != alu2->src[src].abs ||
>>>> +       alu1->src[src].negate != alu2->src[src].negate)
>>>>        return false;
>>>>
>>>> -   for (int i = 0; i < 4; ++i) {
>>>> -      if (!(read_mask & (1 << i)))
>>>> -         continue;
>>>> -
>>>> -      if (src1.swizzle[i] != src2.swizzle[i])
>>>> +   for (unsigned i = 0; i < nir_ssa_alu_instr_src_components(alu1, src); i++) {
>>>> +      if (alu1->src[src].swizzle[i] != alu2->src[src].swizzle[i])
>>>>           return false;
>>>>     }
>>>>
>>>> -   return nir_srcs_equal(src1.src, src2.src);
>>>> +   return nir_srcs_equal(alu1->src[src].src, alu2->src[src].src);
>>>>  }
>>>>
>>>>  static bool
>>>> @@ -74,8 +72,7 @@ nir_instrs_equal(nir_instr *instr1, nir_instr *instr2)
>>>>           return false;
>>>>
>>>>        for (unsigned i = 0; i < nir_op_infos[alu1->op].num_inputs; i++) {
>>>> -         if (!nir_alu_srcs_equal(alu1->src[i], alu2->src[i],
>>>> -                                 (1 << alu1->dest.dest.ssa.num_components) - 1))
>>>> +         if (!nir_alu_srcs_equal(alu1, alu2, i))
>>>>              return false;
>>>>        }
>>>>        return true;
>>>> --
>>>> 2.1.0
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev