[Mesa-dev] [PATCH 6/6] nir: Don't condition 'a-b < 0' -> 'a < b' on is_not_used_by_conditional

[Matt Turner]

On Fri, Mar 23, 2018 at 3:44 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 03/23/2018 11:39 AM, Matt Turner wrote:
>> On Wed, Mar 21, 2018 at 5:58 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>
>>> Now that i965 recognizes that a-b generates the same conditions as 'a <
>>> b', there is no reason to condition this transformation on 'is not used
>>> by conditional.'
>>>
>>> Since this was the only user of the is_not_used_by_conditional function,
>>> delete it.
>>>
>>> All Gen6+ platforms had similar results. (Skylake shown)
>>> total instructions in shared programs: 14400775 -> 14400595 (<.01%)
>>> instructions in affected programs: 36712 -> 36532 (-0.49%)
>>> helped: 182
>>> HURT: 26
>>
>> Looked at why these are hurt? :)
>
> I should have known you were going to ask that. :)  Since the changes
> were small, I tried to avoid looking at them so that I wouldn't get
> distracted again.  But since you asked...
>
> Shaders from 4 apps were hurt.  All but one of the hurt sharders was an
> ARB_fragment_program.  I looked at a shader from each of the four apps,
> and in all of those cases a ffma / compare with zero became a multiply /
> compare with something else.
>
> We already have a general case of this transformation in last
> optimizations.  So, I tried making this transformation more general (by
> removing the fneg) and added a transformation to strip the fnegs from -a
> < -b.  The results on SKL are below.  I'm not excited about the "max: 27
>  x̄: 2.26".
>
> I think I'd like to land this patch as-is.  I'm also going to play with
> moving all the comparison munging out of late optimizations.  I have
> some patches in the queue that add some more transformations, so it will
> be interesting to see the effect of moving them earlier.

Okay. Works for me.

Reviewed-by: Matt Turner <mattst88 at gmail.com>