[Mesa-dev] [PATCH 1/2] i965: Use nir_lower_load_const_to_scalar().

[Jason Ekstrand]

On Jan 23, 2016 12:19 AM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:
>
> On Friday, January 22, 2016 11:54:22 PM PST Jason Ekstrand wrote:
> > On Jan 21, 2016 4:37 PM, "Kenneth Graunke" <kenneth at whitecape.org>
wrote:
> > >
> > > I don't know why, but we never hooked up this pass Eric wrote.
> > > Otherwise, you can end up with stupid scalarized code such as:
> > >
> > >    vec4 ssa_7 = load_const (0.0, 0.0, 0.0, 0.0)
> > >    vec4 ssa_8 = ...
> > >    vec1 ssa_9 = feq ssa_8, ssa_7
> > >    vec1 ssa_10 = feq ssa_8.y, ssa_7.y
> > >    vec1 ssa_11 = feq ssa_8, ssa_7.z
> > >    vec1 ssa_12 = feq ssa_8.y, ssa_7.w
> > >
> > > ssa_8.xyxy == <0, 0, 0, 0> should only take two feq instructions.
> > >
> > > shader-db on Skylake:
> > >
> > > total instructions in shared programs: 9111788 -> 9111384 (-0.00%)
> > > instructions in affected programs: 32421 -> 32017 (-1.25%)
> > > helped: 277
> > > HURT: 69
> > >
> > > total cycles in shared programs: 69221226 -> 69219394 (-0.00%)
> > > cycles in affected programs: 917796 -> 915964 (-0.20%)
> > > helped: 317
> > > HURT: 408
> > >
> > > This also prevents regressions when disabling channel expressions.
> > >
> > > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_nir.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
> > b/src/mesa/drivers/dri/i965/brw_nir.c
> > > index 935529a..ce9b9db 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > > @@ -482,6 +482,11 @@ brw_preprocess_nir(nir_shader *nir, bool
is_scalar)
> > >
> > >     nir = nir_optimize(nir, is_scalar);
> > >
> > > +   if (is_scalar) {
> > > +      OPT_V(nir_lower_load_const_to_scalar);
> > > +      OPT(nir_opt_cse);
> > > +   }
> >
> > Why did you choose to put this *after* the opt loop?  It seems like we
> > would want it before so that we can use better.  As long as
alu_to_scalar
> > is run before constant folding (so we don't end up re-vectorizing them),
> > before should be fine.
>
> It's after the first invocation of the optimization loop, but before the
> second (which isn't obvious from the diff).
>
> I wanted it after nir_lower_vars_to_ssa so that constant initializers
> for variables actually get turned into load_const instructions first.
> Otherwise, we miss splitting them altogether.

That makes sense.  We can't really put it in the loop since it could fight
with vecN constant folding.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160123/39e55c0d/attachment.html>