[Mesa-dev] [PATCH] nir: add nir_instr_type_tex support to nir_lower_phis_to_scalar()

[Jason Ekstrand]

On Fri, Feb 22, 2019 at 9:51 AM Eric Anholt <eric at anholt.net> wrote:

> Timothy Arceri <tarceri at itsqueeze.com> writes:
>
> > shader-db results i965 (SKL):
> >
> > total instructions in shared programs: 13219105 -> 13024761 (-1.47%)
> > instructions in affected programs: 1169457 -> 975113 (-16.62%)
> > helped: 599
> > HURT: 154
> >
> > total cycles in shared programs: 333968972 -> 324822073 (-2.74%)
> > cycles in affected programs: 130032440 -> 120885541 (-7.03%)
> > helped: 590
> > HURT: 216
> >
> > total spills in shared programs: 57947 -> 29130 (-49.73%)
> > spills in affected programs: 53364 -> 24547 (-54.00%)
> > helped: 351
> > HURT: 0
> >
> > total fills in shared programs: 51310 -> 25468 (-50.36%)
> > fills in affected programs: 44882 -> 19040 (-57.58%)
> > helped: 351
> > HURT: 0
> > ---
> >  src/compiler/nir/nir_lower_phis_to_scalar.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/compiler/nir/nir_lower_phis_to_scalar.c
> b/src/compiler/nir/nir_lower_phis_to_scalar.c
> > index 16001f73685..f6f702bca15 100644
> > --- a/src/compiler/nir/nir_lower_phis_to_scalar.c
> > +++ b/src/compiler/nir/nir_lower_phis_to_scalar.c
> > @@ -74,6 +74,7 @@ is_phi_src_scalarizable(nir_phi_src *src,
> >        /* A phi is scalarizable if we're going to lower it */
> >        return should_lower_phi(nir_instr_as_phi(src_instr), state);
> >
> > +   case nir_instr_type_tex:
> >     case nir_instr_type_load_const:
> >     case nir_instr_type_ssa_undef:
> >        /* These are trivially scalarizable */
>
> Sounds promising, but I would definitely not describe instr_type_tex as
> "trivially scalarizable" -- could you explain what's going on with this
> patch?
>

I think I can for Intel though I'm not sure how this affects other drivers.

On Intel hardware, we almost always have to combine all the texture sources
into one big message.  Since having more than one source is very common,
this means that we have to make a temporary copy of the sources anyway.
Because we're copying them, having them contiguous (a vector in NIR terms)
doesn't actually gain us anything.  We may as well let NIR scalarize them
and give more freedom to the register allocator and other NIR passes which
may need to clean things up.  We don't want to make the same choice for
destinations as they are required to be contiguous.

Feel free to copy+paste that somewhere.  I agree with Eric that they are
not "trivially scalarizable" but they are safe to scalarize.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190222/3fda63f4/attachment-0001.html>