<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 23, 2016 at 12:22 PM, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
<br>
> On Mar 23, 2016 2:07 AM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br>
>><br>
>> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
>><br>
>> > Many of our optimizations, while great for cutting shaders down to size,<br>
>> > aren't really precision-safe.  This commit tries to flag all of the<br>
>> > inexact floating-point optimizations so they don't get run on values<br>
> that<br>
>> > are flagged "exact".  It's a bit conservative and maybe flags some safe<br>
>> > optimizations as unsafe but that's better than missing one.<br>
>> > ---<br>
>> >  src/compiler/nir/nir_opt_algebraic.py | 156<br>
> +++++++++++++++++-----------------<br>
>> >  1 file changed, 78 insertions(+), 78 deletions(-)<br>
>> ><br>
>> > diff --git a/src/compiler/nir/nir_opt_algebraic.py<br>
> b/src/compiler/nir/nir_opt_algebraic.py<br>
>> > index 3e7ea06..b229f03 100644<br>
>> > --- a/src/compiler/nir/nir_opt_algebraic.py<br>
>> > +++ b/src/compiler/nir/nir_opt_algebraic.py<br>
>> > @@ -61,19 +61,19 @@ optimizations = [<br>
>> >     (('fabs', ('fneg', a)), ('fabs', a)),<br>
>> >     (('iabs', ('iabs', a)), ('iabs', a)),<br>
>> >     (('iabs', ('ineg', a)), ('iabs', a)),<br>
>> > -   (('fadd', a, 0.0), a),<br>
>> > +   (('~fadd', a, 0.0), a),<br>
>> >     (('iadd', a, 0), a),<br>
>> >     (('usadd_4x8', a, 0), a),<br>
>> >     (('usadd_4x8', a, ~0), ~0),<br>
>> > -   (('fadd', ('fmul', a, b), ('fmul', a, c)), ('fmul', a, ('fadd', b,<br>
> c))),<br>
>> > +   (('~fadd', ('fmul', a, b), ('fmul', a, c)), ('fmul', a, ('fadd', b,<br>
> c))),<br>
>> >     (('iadd', ('imul', a, b), ('imul', a, c)), ('imul', a, ('iadd', b,<br>
> c))),<br>
>> > -   (('fadd', ('fneg', a), a), 0.0),<br>
>> > +   (('~fadd', ('fneg', a), a), 0.0),<br>
>> >     (('iadd', ('ineg', a), a), 0),<br>
>> >     (('iadd', ('ineg', a), ('iadd', a, b)), b),<br>
>> >     (('iadd', a, ('iadd', ('ineg', a), b)), b),<br>
>> > -   (('fadd', ('fneg', a), ('fadd', a, b)), b),<br>
>> > -   (('fadd', a, ('fadd', ('fneg', a), b)), b),<br>
>> > -   (('fmul', a, 0.0), 0.0),<br>
>> > +   (('~fadd', ('fneg', a), ('fadd', a, b)), b),<br>
>> > +   (('~fadd', a, ('fadd', ('fneg', a), b)), b),<br>
>> > +   (('~fmul', a, 0.0), 0.0),<br>
>> >     (('imul', a, 0), 0),<br>
>> >     (('umul_unorm_4x8', a, 0), 0),<br>
>> >     (('umul_unorm_4x8', a, ~0), a),<br>
>> > @@ -81,33 +81,33 @@ optimizations = [<br>
>> >     (('imul', a, 1), a),<br>
>> >     (('fmul', a, -1.0), ('fneg', a)),<br>
>> >     (('imul', a, -1), ('ineg', a)),<br>
>> > -   (('ffma', 0.0, a, b), b),<br>
>> > -   (('ffma', a, 0.0, b), b),<br>
>> > -   (('ffma', a, b, 0.0), ('fmul', a, b)),<br>
>> > -   (('ffma', a, 1.0, b), ('fadd', a, b)),<br>
>> > -   (('ffma', 1.0, a, b), ('fadd', a, b)),<br>
>><br>
>> I think 'ffma a 1 b'/'ffma 1 a b' -> 'fadd a b' could be made safe.<br>
><br>
> I think that should be OK.<br>
><br>
>> > -   (('flrp', a, b, 0.0), a),<br>
>> > -   (('flrp', a, b, 1.0), b),<br>
>> > -   (('flrp', a, a, b), a),<br>
>> > -   (('flrp', 0.0, a, b), ('fmul', a, b)),<br>
>> > +   (('~ffma', 0.0, a, b), b),<br>
>> > +   (('~ffma', a, 0.0, b), b),<br>
>> > +   (('~ffma', a, b, 0.0), ('fmul', a, b)),<br>
>> > +   (('~ffma', a, 1.0, b), ('fadd', a, b)),<br>
>> > +   (('~ffma', 1.0, a, b), ('fadd', a, b)),<br>
>> > +   (('~flrp', a, b, 0.0), a),<br>
>> > +   (('~flrp', a, b, 1.0), b),<br>
>> > +   (('~flrp', a, a, b), a),<br>
>> > +   (('~flrp', 0.0, a, b), ('fmul', a, b)),<br>
>> >     (('flrp', a, b, c), ('fadd', ('fmul', c, ('fsub', b, a)), a),<br>
> 'options->lower_flrp'),<br>
>> >     (('ffract', a), ('fsub', a, ('ffloor', a)),<br>
> 'options->lower_ffract'),<br>
>> > -   (('fadd', ('fmul', a, ('fadd', 1.0, ('fneg', c))), ('fmul', b, c)),<br>
> ('flrp', a, b, c), '!options->lower_flrp'),<br>
>> > -   (('fadd', a, ('fmul', c, ('fadd', b, ('fneg', a)))), ('flrp', a, b,<br>
> c), '!options->lower_flrp'),<br>
>> > +   (('~fadd', ('fmul', a, ('fadd', 1.0, ('fneg', c))), ('fmul', b,<br>
> c)), ('flrp', a, b, c), '!options->lower_flrp'),<br>
>> > +   (('~fadd', a, ('fmul', c, ('fadd', b, ('fneg', a)))), ('flrp', a,<br>
> b, c), '!options->lower_flrp'),<br>
>> >     (('ffma', a, b, c), ('fadd', ('fmul', a, b), c),<br>
> 'options->lower_ffma'),<br>
>> > -   (('fadd', ('fmul', a, b), c), ('ffma', a, b, c),<br>
> '!options->lower_ffma'),<br>
>> > +   (('~fadd', ('fmul', a, b), c), ('ffma', a, b, c),<br>
> '!options->lower_ffma'),<br>
>> >     # Comparison simplifications<br>
>> > -   (('inot', ('flt', a, b)), ('fge', a, b)),<br>
>> > -   (('inot', ('fge', a, b)), ('flt', a, b)),<br>
>> > -   (('inot', ('feq', a, b)), ('fne', a, b)),<br>
>> > -   (('inot', ('fne', a, b)), ('feq', a, b)),<br>
>> > -   (('inot', ('ilt', a, b)), ('ige', a, b)),<br>
>> > -   (('inot', ('ige', a, b)), ('ilt', a, b)),<br>
>> > -   (('inot', ('ieq', a, b)), ('ine', a, b)),<br>
>> > -   (('inot', ('ine', a, b)), ('ieq', a, b)),<br>
>><br>
>> What's unsafe about the four integer comparison simplifications above?<br>
><br>
> Right.  I got too excited.  I'll leave those as safe.<br>
><br>
>> > -   (('fge', ('fneg', ('fabs', a)), 0.0), ('feq', a, 0.0)),<br>
>><br>
>> This one seems mostly safe too.<br>
>><br>
>> > -   (('bcsel', ('flt', a, b), a, b), ('fmin', a, b)),<br>
>> > -   (('bcsel', ('flt', a, b), b, a), ('fmax', a, b)),<br>
>><br>
>> What are the semantics of fmin/fmax?  Are they defined as in GLSL with<br>
>> min(NaN, x) = NaN, min(x, NaN) = x?  Or the other way around?  Or is it<br>
>> defined to be commutative?  In the first case the code above would<br>
>> indeed be unsafe *but* the related simplification:<br>
><br>
> I dug up the GLSL spec and it says the following for min:<br>
><br>
> Returns y if y < x, otherwise it returns x<br>
><br>
> so min(NaN, y) = y but min(x, NaN) = NaN<br>
><br>
> If we just change the optimizations to match the spec better, they would be<br>
> safe.  Then we could add the non-safe ones as non-safe.<br>
><br>
</div></div>They wouldn't, you still need the two-character change I pointed out<br>
earlier (marked with '^^^^^' now) to make it safe.<br>
<span class=""><br>
> Sadly, I think our hardware implements the DX behaviour of "give me the<br>
> non-NaN value"<br>
><br>
</span>Our hardware can do either, CMPN and SEL.l/ge gives you the non-NaN<br>
value while CMP and SEL.le/g gives you GLSL-like semantics.  The<br>
back-end uses the wrong ones almost universally since<br>
3b7f683f3bbbd93e417a6f42ec7c609465be49de except on Gen4-5 which use CMP.<br>
<br>
                                                   vvvv<br>
<span class="">>> |     (('bcsel', ('flt', a, b), a, b), ('fmin', b, a)),<br>
</span>                                                   ^^^^^<br>
<div><div class="h5">>> |     (('bcsel', ('flt', a, b), b, a), ('fmax', a, b)),<br>
>><br>
>> would be completely safe.<br>
>><br>
>> > +   (('~inot', ('flt', a, b)), ('fge', a, b)),<br>
>> > +   (('~inot', ('fge', a, b)), ('flt', a, b)),<br>
>> > +   (('~inot', ('feq', a, b)), ('fne', a, b)),<br>
>> > +   (('~inot', ('fne', a, b)), ('feq', a, b)),<br>
>> > +   (('~inot', ('ilt', a, b)), ('ige', a, b)),<br>
>> > +   (('~inot', ('ige', a, b)), ('ilt', a, b)),<br>
>> > +   (('~inot', ('ieq', a, b)), ('ine', a, b)),<br>
>> > +   (('~inot', ('ine', a, b)), ('ieq', a, b)),<br>
>> > +   (('~fge', ('fneg', ('fabs', a)), 0.0), ('feq', a, 0.0)),<br>
>> > +   (('~bcsel', ('flt', a, b), a, b), ('fmin', a, b)),<br>
>> > +   (('~bcsel', ('flt', a, b), b, a), ('fmax', a, b)),<br>
>> >     (('bcsel', ('inot', 'a@bool'), b, c), ('bcsel', a, c, b)),<br>
>> >     (('bcsel', a, ('bcsel', a, b, c), d), ('bcsel', a, b, d)),<br>
>> >     (('fmin', a, a), a),<br>
>> > @@ -116,17 +116,17 @@ optimizations = [<br>
>> >     (('imax', a, a), a),<br>
>> >     (('umin', a, a), a),<br>
>> >     (('umax', a, a), a),<br>
>> > -   (('fmin', ('fmax', a, 0.0), 1.0), ('fsat', a),<br>
> '!options->lower_fsat'),<br>
>> > -   (('fmax', ('fmin', a, 1.0), 0.0), ('fsat', a),<br>
> '!options->lower_fsat'),<br>
>> > +   (('~fmin', ('fmax', a, 0.0), 1.0), ('fsat', a),<br>
> '!options->lower_fsat'),<br>
>> > +   (('~fmax', ('fmin', a, 1.0), 0.0), ('fsat', a),<br>
> '!options->lower_fsat'),<br>
>> >     (('fsat', a), ('fmin', ('fmax', a, 0.0), 1.0),<br>
> 'options->lower_fsat'),<br>
>> >     (('fsat', ('fsat', a)), ('fsat', a)),<br>
>> > -   (('fsat', ('fmin', a, 1.0)), ('fsat', a)),<br>
>> > -   (('fsat', ('fmax', a, 0.0)), ('fsat', a)),<br>
>> > +   (('~fsat', ('fmin', a, 1.0)), ('fsat', a)),<br>
>> > +   (('~fsat', ('fmax', a, 0.0)), ('fsat', a)),<br>
>> >     (('fmin', ('fmax', ('fmin', ('fmax', a, 0.0), 1.0), 0.0), 1.0),<br>
> ('fmin', ('fmax', a, 0.0), 1.0)),<br>
>> > -   (('ior', ('flt', a, b), ('flt', a, c)), ('flt', a, ('fmax', b, c))),<br>
>> > -   (('ior', ('flt', a, c), ('flt', b, c)), ('flt', ('fmin', a, b), c)),<br>
>> > -   (('ior', ('fge', a, b), ('fge', a, c)), ('fge', a, ('fmin', b, c))),<br>
>> > -   (('ior', ('fge', a, c), ('fge', b, c)), ('fge', ('fmax', a, b), c)),<br>
>><br>
>> These would be safe if fmin/fmax were commutative, which I don't think<br>
>> is the case?<br>
><br>
> They are not commutative :-(<br>
><br>
>> > +   (('~ior', ('flt', a, b), ('flt', a, c)), ('flt', a, ('fmax', b,<br>
> c))),<br>
>> > +   (('~ior', ('flt', a, c), ('flt', b, c)), ('flt', ('fmin', a, b),<br>
> c)),<br>
>> > +   (('~ior', ('fge', a, b), ('fge', a, c)), ('fge', a, ('fmin', b,<br>
> c))),<br>
>> > +   (('~ior', ('fge', a, c), ('fge', b, c)), ('fge', ('fmax', a, b),<br>
> c)),<br>
>> >     (('slt', a, b), ('b2f', ('flt', a, b)), 'options->lower_scmp'),<br>
>> >     (('sge', a, b), ('b2f', ('fge', a, b)), 'options->lower_scmp'),<br>
>> >     (('seq', a, b), ('b2f', ('feq', a, b)), 'options->lower_scmp'),<br>
>> > @@ -150,15 +150,15 @@ optimizations = [<br>
>> >     (('ult', a, a), False),<br>
>> >     (('uge', a, a), True),<br>
>> >     # Logical and bit operations<br>
>> > -   (('fand', a, 0.0), 0.0),<br>
>> > +   (('~fand', a, 0.0), 0.0),<br>
>><br>
>> This is safe if fand(x, y) is defined as (x != 0 && y != 0 ? 1 : 0).<br>
>> Could you remind me how it is defined?<br>
><br>
> Yes, that is the definition.<br>
><br>
>> >     (('iand', a, a), a),<br>
>> >     (('iand', a, ~0), a),<br>
>> >     (('iand', a, 0), 0),<br>
>> >     (('ior', a, a), a),<br>
>> >     (('ior', a, 0), a),<br>
>> > -   (('fxor', a, a), 0.0),<br>
>> > +   (('~fxor', a, a), 0.0),<br>
>><br>
>> Same goes here.<br>
><br>
> Right.<br>
><br>
>> >     (('ixor', a, a), 0),<br>
>> > -   (('fxor', a, 0.0), a),<br>
>> > +   (('~fxor', a, 0.0), a),<br>
>> >     (('ixor', a, 0), a),<br>
>> >     (('inot', ('inot', a)), a),<br>
>> >     # DeMorgan's Laws<br>
>> > @@ -174,35 +174,35 @@ optimizations = [<br>
>> >     (('iand', 0xff, ('ushr', a, 24)), ('ushr', a, 24)),<br>
>> >     (('iand', 0xffff, ('ushr', a, 16)), ('ushr', a, 16)),<br>
>> >     # Exponential/logarithmic identities<br>
>> > -   (('fexp2', ('flog2', a)), a), # 2^lg2(a) = a<br>
>> > -   (('flog2', ('fexp2', a)), a), # lg2(2^a) = a<br>
>> > +   (('~fexp2', ('flog2', a)), a), # 2^lg2(a) = a<br>
>> > +   (('~flog2', ('fexp2', a)), a), # lg2(2^a) = a<br>
>> >     (('fpow', a, b), ('fexp2', ('fmul', ('flog2', a), b)),<br>
> 'options->lower_fpow'), # a^b = 2^(lg2(a)*b)<br>
>> > -   (('fexp2', ('fmul', ('flog2', a), b)), ('fpow', a, b),<br>
> '!options->lower_fpow'), # 2^(lg2(a)*b) = a^b<br>
>> > -   (('fexp2', ('fadd', ('fmul', ('flog2', a), b), ('fmul', ('flog2',<br>
> c), d))),<br>
>> > -    ('fmul', ('fpow', a, b), ('fpow', c, d)), '!options->lower_fpow'),<br>
> # 2^(lg2(a) * b + lg2(c) + d) = a^b * c^d<br>
>> > -   (('fpow', a, 1.0), a),<br>
>> > -   (('fpow', a, 2.0), ('fmul', a, a)),<br>
>> > -   (('fpow', a, 4.0), ('fmul', ('fmul', a, a), ('fmul', a, a))),<br>
>> > -   (('fpow', 2.0, a), ('fexp2', a)),<br>
>> > -   (('fpow', ('fpow', a, 2.2), 0.454545), a),<br>
>> > -   (('fpow', ('fabs', ('fpow', a, 2.2)), 0.454545), ('fabs', a)),<br>
>> > -   (('fsqrt', ('fexp2', a)), ('fexp2', ('fmul', 0.5, a))),<br>
>> > -   (('frcp', ('fexp2', a)), ('fexp2', ('fneg', a))),<br>
>> > -   (('frsq', ('fexp2', a)), ('fexp2', ('fmul', -0.5, a))),<br>
>> > -   (('flog2', ('fsqrt', a)), ('fmul', 0.5, ('flog2', a))),<br>
>> > -   (('flog2', ('frcp', a)), ('fneg', ('flog2', a))),<br>
>> > -   (('flog2', ('frsq', a)), ('fmul', -0.5, ('flog2', a))),<br>
>> > -   (('flog2', ('fpow', a, b)), ('fmul', b, ('flog2', a))),<br>
>> > -   (('fadd', ('flog2', a), ('flog2', b)), ('flog2', ('fmul', a, b))),<br>
>> > -   (('fadd', ('flog2', a), ('fneg', ('flog2', b))), ('flog2', ('fdiv',<br>
> a, b))),<br>
>> > -   (('fmul', ('fexp2', a), ('fexp2', b)), ('fexp2', ('fadd', a, b))),<br>
>> > +   (('~fexp2', ('fmul', ('flog2', a), b)), ('fpow', a, b),<br>
> '!options->lower_fpow'), # 2^(lg2(a)*b) = a^b<br>
>> > +   (('~fexp2', ('fadd', ('fmul', ('flog2', a), b), ('fmul', ('flog2',<br>
> c), d))),<br>
>> > +    ('~fmul', ('fpow', a, b), ('fpow', c, d)),<br>
> '!options->lower_fpow'), # 2^(lg2(a) * b + lg2(c) + d) = a^b * c^d<br>
>> > +   (('~fpow', a, 1.0), a),<br>
>> > +   (('~fpow', a, 2.0), ('fmul', a, a)),<br>
>> > +   (('~fpow', a, 4.0), ('fmul', ('fmul', a, a), ('fmul', a, a))),<br>
>> > +   (('~fpow', 2.0, a), ('fexp2', a)),<br>
>> > +   (('~fpow', ('fpow', a, 2.2), 0.454545), a),<br>
>> > +   (('~fpow', ('fabs', ('fpow', a, 2.2)), 0.454545), ('fabs', a)),<br>
>> > +   (('~fsqrt', ('fexp2', a)), ('fexp2', ('fmul', 0.5, a))),<br>
>> > +   (('~frcp', ('fexp2', a)), ('fexp2', ('fneg', a))),<br>
>> > +   (('~frsq', ('fexp2', a)), ('fexp2', ('fmul', -0.5, a))),<br>
>> > +   (('~flog2', ('fsqrt', a)), ('fmul', 0.5, ('flog2', a))),<br>
>> > +   (('~flog2', ('frcp', a)), ('fneg', ('flog2', a))),<br>
>> > +   (('~flog2', ('frsq', a)), ('fmul', -0.5, ('flog2', a))),<br>
>> > +   (('~flog2', ('fpow', a, b)), ('fmul', b, ('flog2', a))),<br>
>> > +   (('~fadd', ('flog2', a), ('flog2', b)), ('flog2', ('fmul', a, b))),<br>
>> > +   (('~fadd', ('flog2', a), ('fneg', ('flog2', b))), ('flog2',<br>
> ('fdiv', a, b))),<br>
>> > +   (('~fmul', ('fexp2', a), ('fexp2', b)), ('fexp2', ('fadd', a, b))),<br>
>> >     # Division and reciprocal<br>
>> > -   (('fdiv', 1.0, a), ('frcp', a)),<br>
>> > -   (('fdiv', a, b), ('fmul', a, ('frcp', b)), 'options->lower_fdiv'),<br>
>><br>
>> This one is safe and I guess necessary if the back-end requires fdiv to<br>
>> be lowered?<br>
><br>
> Thanks. I must have gotten too eager to flag things.<br>
><br>
>> > -   (('frcp', ('frcp', a)), a),<br>
>> > -   (('frcp', ('fsqrt', a)), ('frsq', a)),<br>
>> > -   (('fsqrt', a), ('frcp', ('frsq', a)), 'options->lower_fsqrt'),<br>
>><br>
>> Same here.<br>
><br>
> Yup<br>
><br>
>> > -   (('frcp', ('frsq', a)), ('fsqrt', a), '!options->lower_fsqrt'),<br>
>> > +   (('~fdiv', 1.0, a), ('frcp', a)),<br>
>> > +   (('~fdiv', a, b), ('fmul', a, ('frcp', b)), 'options->lower_fdiv'),<br>
>> > +   (('~frcp', ('frcp', a)), a),<br>
>> > +   (('~frcp', ('fsqrt', a)), ('frsq', a)),<br>
>> > +   (('~fsqrt', a), ('frcp', ('frsq', a)), 'options->lower_fsqrt'),<br>
>> > +   (('~frcp', ('frsq', a)), ('fsqrt', a), '!options->lower_fsqrt'),<br>
>> >     # Boolean simplifications<br>
>> >     (('ieq', 'a@bool', True), a),<br>
>> >     (('ine', 'a@bool', True), ('inot', a)),<br>
>> > @@ -221,8 +221,8 @@ optimizations = [<br>
>> ><br>
>> >     # Conversions<br>
>> >     (('i2b', ('b2i', a)), a),<br>
>> > -   (('f2i', ('ftrunc', a)), ('f2i', a)),<br>
>> > -   (('f2u', ('ftrunc', a)), ('f2u', a)),<br>
>> > +   (('~f2i', ('ftrunc', a)), ('f2i', a)),<br>
>> > +   (('~f2u', ('ftrunc', a)), ('f2u', a)),<br>
>> ><br>
>> These seem safe to me.<br>
> I think they probably are.  I think there may be a problem when the number<br>
> of mantissa bits is bigger than the number of integer bits.  In that case,<br>
> the out-of-bounds behaviour may get interesting.  However, we're not<br>
> handling that case yet so I think it's safe.<br>
><br>
</div></div>Hm, I don't think I understand the problem, if the integer part of 'a'<br>
is outside the representable range of the integer type, is that not<br>
going to be the case still after 'ftrunc()'?<br><div><div class="h5"></div></div></blockquote><div><br></div><div>Like I said, I think it's safe.  I was mostly pointing out a place where I think there *may* be a problem if we had different bit sizes involved.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
>> >     # Byte extraction<br>
>> >     (('ushr', a, 24), ('extract_u8', a, 3),<br>
> '!options->lower_extract_byte'),<br>
>> > @@ -235,17 +235,17 @@ optimizations = [<br>
>> >     (('iand', 0xffff, a), ('extract_u16', a, 0),<br>
> '!options->lower_extract_word'),<br>
>> ><br>
>> >     # Subtracts<br>
>> > -   (('fsub', a, ('fsub', 0.0, b)), ('fadd', a, b)),<br>
>> > +   (('~fsub', a, ('fsub', 0.0, b)), ('fadd', a, b)),<br>
>> >     (('isub', a, ('isub', 0, b)), ('iadd', a, b)),<br>
>> >     (('ussub_4x8', a, 0), a),<br>
>> >     (('ussub_4x8', a, ~0), 0),<br>
>> > -   (('fsub', a, b), ('fadd', a, ('fneg', b)), 'options->lower_sub'),<br>
>> > +   (('~fsub', a, b), ('fadd', a, ('fneg', b)), 'options->lower_sub'),<br>
>><br>
>> Required for lowering.<br>
><br>
> Yup<br>
><br>
>> >     (('isub', a, b), ('iadd', a, ('ineg', b)), 'options->lower_sub'),<br>
>> > -   (('fneg', a), ('fsub', 0.0, a), 'options->lower_negate'),<br>
>> > +   (('~fneg', a), ('fsub', 0.0, a), 'options->lower_negate'),<br>
>><br>
>> Same here.<br>
><br>
> Yup<br>
><br>
>> >     (('ineg', a), ('isub', 0, a), 'options->lower_negate'),<br>
>> > -   (('fadd', a, ('fsub', 0.0, b)), ('fsub', a, b)),<br>
>> > +   (('~fadd', a, ('fsub', 0.0, b)), ('fsub', a, b)),<br>
>> >     (('iadd', a, ('isub', 0, b)), ('isub', a, b)),<br>
>> > -   (('fabs', ('fsub', 0.0, a)), ('fabs', a)),<br>
>> > +   (('~fabs', ('fsub', 0.0, a)), ('fabs', a)),<br>
>><br>
>> This one seems safe.<br>
><br>
> Depends on whether or not a + 0.0 == a.  I think it should but glennk<br>
> seemed to think it wasn't.  I'd rather be on the safe side for now.<br>
<br>
</div></div>Heh, the reason I pointed that out is that because of the fabs() around<br>
the subtraction it doesn't actually matter whether '0 - a == -a' or not.<br>
The only case where the equality may not hold is for a=+0, because<br>
'(+0) - (+0)' might be +0 or -0 depending on the rounding behaviour of the<br>
implementation.  However the end result is the same because fabs(-0) ==<br>
fabs(+0).<span class=""><br></span></blockquote><div><br></div><div>I think that's correct and I'm ok with calling it safe for now.  However, glenn seemed to think it wasn't and I didn't understand his half-explination.  We'll call it safe for now.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
><br>
>> >     (('iabs', ('isub', 0, a)), ('iabs', a)),<br>
>> ><br>
>> >     # Misc. lowering<br>
>> > @@ -372,10 +372,10 @@ for op in ['flt', 'fge', 'feq', 'fne',<br>
>> >  # they help code generation but do not necessarily produce code that is<br>
>> >  # more easily optimizable.<br>
>> >  late_optimizations = [<br>
>> > -   (('flt', ('fadd', a, b), 0.0), ('flt', a, ('fneg', b))),<br>
>><br>
>> This one also seems safe,<br>
> I've come to the conclusion that it isn't.  If you have a -inf and inf,<br>
> then a+b is going to be one of NaN, inf, or 0.  In any case the expression<br>
> on the left is commutative while the one on the right isn't.<br>
><br>
</span>Yeah, well, they may not be equivalent but only if the implementation<br>
doesn't do IEEE-compliant NaN handling and it doesn't flush them to zero<br>
either -- In either of those cases the LHS evaluates to false while the<br>
RHS does too because the two infs have opposite signs.  It might be<br>
useful to add some sort of compiler option for the back-end to specify<br>
its preferred NaN handling rules?<br><div class="HOEnZb"><div class="h5"></div></div></blockquote><div><br></div><div>Wow... Ok, I misred the optimization...  I think it's probably is safe.  I'd be tempted to leave it for the sake of consistency because I'd be afraid that someone will come along later and say "oops, we missed one".  I'll leave a comment so that doesn't happen.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
>> > -   (('fge', ('fadd', a, b), 0.0), ('fge', a, ('fneg', b))),<br>
>> > -   (('feq', ('fadd', a, b), 0.0), ('feq', a, ('fneg', b))),<br>
>> > -   (('fne', ('fadd', a, b), 0.0), ('fne', a, ('fneg', b))),<br>
>><br>
>> ...but these other ones are indeed unsafe for some sign combinations of<br>
>> a and b when they are not finite.<br>
>><br>
>> > +   (('~flt', ('fadd', a, b), 0.0), ('flt', a, ('fneg', b))),<br>
>> > +   (('~fge', ('fadd', a, b), 0.0), ('fge', a, ('fneg', b))),<br>
>> > +   (('~feq', ('fadd', a, b), 0.0), ('feq', a, ('fneg', b))),<br>
>> > +   (('~fne', ('fadd', a, b), 0.0), ('fne', a, ('fneg', b))),<br>
>> >     (('fdot2', a, b), ('fdot_replicated2', a, b),<br>
> 'options->fdot_replicates'),<br>
>> >     (('fdot3', a, b), ('fdot_replicated3', a, b),<br>
> 'options->fdot_replicates'),<br>
>> >     (('fdot4', a, b), ('fdot_replicated4', a, b),<br>
> 'options->fdot_replicates'),<br>
>><br>
>> I've also looked through the remaining algebraic optimizations in this<br>
>> pass, and things look safe except for the ones you have already marked<br>
>> unsafe in this patch -- I'll wait until you answer the questions I<br>
>> pointed out before I send you a R-b for it though.<br>
>><br>
>> > --<br>
>> > 2.5.0.400.gff86faf<br>
>> ><br>
>> > _______________________________________________<br>
>> > mesa-dev mailing list<br>
>> > <a href="mailto:mesa-dev@lists.freedesktop.org">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>
</div></div></blockquote></div><br></div></div>