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

[Jason Ekstrand]

On Tue, Apr 14, 2015 at 2:00 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> 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).

Right...

> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150414/7dca82ce/attachment-0001.html>