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

[Timothy Arceri]

On Tue, 2015-11-03 at 20:42 -0500, Ilia Mirkin wrote:
> 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.


Hi Jason,

Since you wrote the nir_lower_vars_to_ssa pass do you have any comments on
this? If we end up with a case were we have have var = var should this pass
just not try to lower it rather than hitting the assert, should we try remove
it here? Or should we try to removing it in other optisation passes?

My patch removes it in the GLSL IR for the scenario it was dicovered for, but
as Ilia pointed out it could be possible to hit it as the results of
optimisation passes in nir.

Thanks,
Tim

 


> 
>   -ilia