<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Feb 22, 2019 at 2:47 PM Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 23/2/19 6:31 am, Rob Clark wrote:<br>
> On Fri, Feb 22, 2019 at 12:39 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
>><br>
>> On Fri, Feb 22, 2019 at 9:51 AM Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote:<br>
>>><br>
>>> Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>> writes:<br>
>>><br>
>>>> shader-db results i965 (SKL):<br>
>>>><br>
>>>> total instructions in shared programs: 13219105 -> 13024761 (-1.47%)<br>
>>>> instructions in affected programs: 1169457 -> 975113 (-16.62%)<br>
>>>> helped: 599<br>
>>>> HURT: 154<br>
>>>><br>
>>>> total cycles in shared programs: 333968972 -> 324822073 (-2.74%)<br>
>>>> cycles in affected programs: 130032440 -> 120885541 (-7.03%)<br>
>>>> helped: 590<br>
>>>> HURT: 216<br>
>>>><br>
>>>> total spills in shared programs: 57947 -> 29130 (-49.73%)<br>
>>>> spills in affected programs: 53364 -> 24547 (-54.00%)<br>
>>>> helped: 351<br>
>>>> HURT: 0<br>
>>>><br>
>>>> total fills in shared programs: 51310 -> 25468 (-50.36%)<br>
>>>> fills in affected programs: 44882 -> 19040 (-57.58%)<br>
>>>> helped: 351<br>
>>>> HURT: 0<br>
>>>> ---<br>
>>>>   src/compiler/nir/nir_lower_phis_to_scalar.c | 1 +<br>
>>>>   1 file changed, 1 insertion(+)<br>
>>>><br>
>>>> diff --git a/src/compiler/nir/nir_lower_phis_to_scalar.c b/src/compiler/nir/nir_lower_phis_to_scalar.c<br>
>>>> index 16001f73685..f6f702bca15 100644<br>
>>>> --- a/src/compiler/nir/nir_lower_phis_to_scalar.c<br>
>>>> +++ b/src/compiler/nir/nir_lower_phis_to_scalar.c<br>
>>>> @@ -74,6 +74,7 @@ is_phi_src_scalarizable(nir_phi_src *src,<br>
>>>>         /* A phi is scalarizable if we're going to lower it */<br>
>>>>         return should_lower_phi(nir_instr_as_phi(src_instr), state);<br>
>>>><br>
>>>> +   case nir_instr_type_tex:<br>
>>>>      case nir_instr_type_load_const:<br>
>>>>      case nir_instr_type_ssa_undef:<br>
>>>>         /* These are trivially scalarizable */<br>
>>><br>
>>> Sounds promising, but I would definitely not describe instr_type_tex as<br>
>>> "trivially scalarizable" -- could you explain what's going on with this<br>
>>> patch?<br>
<br>
Basically it just turns:<br>
<br>
if ssa0 {<br>
   ...<br>
   vec4 ssa1 = txf .....<br>
} else {<br>
    ...<br>
    vec4 ssa2 = ...<br>
}<br>
vec4 ss3 = phi ssa1, ssa2<br>
<br>
Into<br>
<br>
if ssa0 {<br>
   ...<br>
   vec4 ssa1 = txf .....<br>
   vec1 ssa2 = imov ssa1.x<br>
   vec1 ssa3 = imov ssa1.y<br>
   vec1 ssa4 = imov ssa1.z<br>
   vec1 ssa5 = imov ssa1.w<br>
} else {<br>
    ...<br>
    vec4 ssa6 = ...<br>
    vec1 ssa7 = imov ssa6.x<br>
    vec1 ssa8 = imov ssa6.y<br>
    vec1 ssa9 = imov ssa6.z<br>
    vec1 ssa10 = imov ssa6.w<br>
}<br>
vec1 ss11 = phi ssa2, ssa7<br>
vec1 ss12 = phi ssa3, ssa8<br>
vec1 ss13 = phi ssa4, ssa9<br>
vec1 ss14 = phi ssa5, ssa10<br>
<br>
<br>
This allows a whole bunch more optimisation to take place as often not <br>
all of the phi channels are actually used. If some cases large chunks of <br>
logic can be remove from the if branch that doesn't contain the texture <br>
access.<br>
<br>
>><br>
>><br>
>> I think I can for Intel though I'm not sure how this affects other drivers.<br>
>><br>
>> 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.<br>
>><br>
> <br>
> hmm, but this is abut the phi src (ie. the tex dest), not the tex src, isn't it?<br></blockquote><div><br></div><div>Well, that's a pickle...  When I wrote this pass I did so to try and explicitly keep from breaking up things that are known to be vectors in the back-end such as textures.  The idea was to try and not break up things like this:</div><div><br></div><div> if (...) {</div><div>    ssa_1 = tex()</div><div>} else {</div><div>    ssa_2 = tex()</div><div>}</div><div>ssa_3 = phi (ssa_1, ssa_2)</div><div><br></div><div>in the hopes that it would turn into</div><div><br></div><div>if (...) {</div><div>    r1 = tex();</div><div>} else {</div><div>    r1 = tex();</div><div>}</div><div><br></div><div>Clearly, that notion was mis-placed.  At this point, I really wonder what the complexity is saving us.  Maybe it's not worth it at all?  Maybe we need to be more agressive and require all sources to not be vectorizable or something?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Yeah, thats correct. Which is why I think its fine to be bunched with <br>
the "These are trivially scalarizable" types?<br>
<br>
> <br>
> BR,<br>
> -R<br>
> <br>
>> Feel free to copy+paste that somewhere.  I agree with Eric that they are not "trivially scalarizable" but they are safe to scalarize.<br>
>><br>
>> --Jason<br>
>> _______________________________________________<br>
>> mesa-dev mailing list<br>
>> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
>> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>