On Fri, Feb 22, 2019 at 3:47 PM Timothy Arceri <tarc...@itsqueeze.com> wrote:
>
>
>
> On 23/2/19 6:31 am, Rob Clark wrote:
> > On Fri, Feb 22, 2019 at 12:39 PM Jason Ekstrand <ja...@jlekstrand.net>
> > wrote:
> >>
> >> On Fri, Feb 22, 2019 at 9:51 AM Eric Anholt <e...@anholt.net> wrote:
> >>>
> >>> Timothy Arceri <tarc...@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?
>
> Basically it just turns:
>
> if ssa0 {
> ...
> vec4 ssa1 = txf .....
> } else {
> ...
> vec4 ssa2 = ...
> }
> vec4 ss3 = phi ssa1, ssa2
>
> Into
>
> if ssa0 {
> ...
> vec4 ssa1 = txf .....
> vec1 ssa2 = imov ssa1.x
> vec1 ssa3 = imov ssa1.y
> vec1 ssa4 = imov ssa1.z
> vec1 ssa5 = imov ssa1.w
> } else {
> ...
> vec4 ssa6 = ...
> vec1 ssa7 = imov ssa6.x
> vec1 ssa8 = imov ssa6.y
> vec1 ssa9 = imov ssa6.z
> vec1 ssa10 = imov ssa6.w
> }
> vec1 ss11 = phi ssa2, ssa7
> vec1 ss12 = phi ssa3, ssa8
> vec1 ss13 = phi ssa4, ssa9
> vec1 ss14 = phi ssa5, ssa10
ok, I didn't realize this is how the tex would get scalarized, I was
more concerned that it would get turned into something like:
vec1 ssa1 = txf.x ...
vec1 ssa2 = txf.y ...
vec1 ssa3 = txf.z ...
vec1 ssa4 = txf.w ...
(which might look good in shaderdb but really would be sub-optimal)
BR,
-R
>
>
> This allows a whole bunch more optimisation to take place as often not
> all of the phi channels are actually used. If some cases large chunks of
> logic can be remove from the if branch that doesn't contain the texture
> access.
>
> >>
> >>
> >> 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.
> >>
> >
> > hmm, but this is abut the phi src (ie. the tex dest), not the tex src,
> > isn't it?
>
> Yeah, thats correct. Which is why I think its fine to be bunched with
> the "These are trivially scalarizable" types?
>
> >
> > BR,
> > -R
> >
> >> Feel free to copy+paste that somewhere. I agree with Eric that they are
> >> not "trivially scalarizable" but they are safe to scalarize.
> >>
> >> --Jason
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
