[Mesa-dev] [PATCH] glsl: remove redundant function inout assignments

Ilia Mirkin imirkin at alum.mit.edu
Tue Nov 3 17:42:06 PST 2015


On Tue, Nov 3, 2015 at 8:23 PM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> On Wed, 2015-11-04 at 12:12 +1100, Timothy Arceri wrote:
>> On Tue, 2015-11-03 at 19:39 -0500, Ilia Mirkin wrote:
>> > On Tue, Nov 3, 2015 at 7:31 PM, Timothy Arceri <t_arceri at yahoo.com.au>
>> > wrote:
>> > > On Tue, 2015-11-03 at 19:21 -0500, Ilia Mirkin wrote:
>> > > > I'm still unclear what problem you're trying to solve here? What's
>> > > > wrong with having b[1] = b[1]?
>> > >
>> > > There is nothing wrong with it, but nir seems to expect this type of
>> > > assignment to be optimised out and hits an assert if its not. Its
>> > > removed
>> > > for
>> > > non arrays in opt_copy_propagation.cpp see add_copy()
>> >
>> > Sounds like a bugfix for nir might be in order then? What if you have,
>> > say, b[1] = b[1] + 0 and then NIR optimizes away the + 0? Or +
>> > some_var which happens to become 0 as a result of optimizations?
>>
>> Assuming the GLSL IR doesn't get to it first your right, however this is
>> tripping up nir_lower_vars_to_ssa() which is the first optimisation call for
>> nir drivers, so we still need to add this to the GLSL IR optimisations
>> unless
>> there is an argument to move the lower pass until after the other
>> optimisation
>> passes but I assume it was added to the beginning for a reason.
>>
>> I moved the lower call to the end of the optimisation list to see what
>> happens
>> and it does indeed still fail. I can have a go at adding the optimisation to
>> nir when I get some time
>
> I can also note this in the bug report and leave it open until it is resolved.
>
>>  but I don't think this should be a blocker to the
>> GLSL IR fix.

Perhaps you should wait for someone with a better understanding of
what's going on than me to review.

It just seems odd to have this super-special-cased thing in a GLSL IR
opt just because of some generic-looking NIR deficiency. It sounds
like the sort of thing that'd be very easy to fix there, instead of
adding seemingly unnecessary extra work in the GLSL IR.

  -ilia


More information about the mesa-dev mailing list