[Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Ian Romanick]

On 11/29/2018 07:47 AM, Connor Abbott wrote:
> On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand <jason at jlekstrand.net> wrote:
>>
>> Can you provide some context for this?  Those rules are already flagged "inexact" (that's what the ~ means) so they won't apply to anything that's "precise" or "invariant".
> 
> I think the concern is that this isn't allowed in SPIR-V, even without
> exact or invariant. We even go out of our way to do the correct thing
> in the frontend by inserting an "&& a == a" or "|| a != a", but then

If you're that paranoid about it, why not just mark the operations are
precise?  That's literally why it exists.

> opt_algebraic removes it with another rule and then this rule can flip
> it from ordered to unordered. The spec says that operations don't have
> to produce NaN, but it doesn't say anything on comparisons other than
> the generic "everything must follow IEEE rules" and an entry in the
> table that says "produces correct results." Then again, I can't find
> anything in GLSL allowing these transforms either, so maybe we just
> need to get rid of them.

What I hear you saying is, "The behavior isn't defined."  Unless you can
point to a CTS test or an application that has incorrect behavior, I'm
going to oppose removing this pretty strongly.  *Every* GLSL compiler
does this.

>> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset <samuel.pitoiset at gmail.com> wrote:
>>>
>>> It's correct in GLSL because the behaviour is undefined in
>>> presence of NaNs. But this seems incorrect in Vulkan.
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>>  src/compiler/nir/nir.h                | 6 ++++++
>>>  src/compiler/nir/nir_opt_algebraic.py | 8 ++++----
>>>  2 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>>> index db935c8496b..4107c293962 100644
>>> --- a/src/compiler/nir/nir.h
>>> +++ b/src/compiler/nir/nir.h
>>> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
>>>     /* Set if nir_lower_wpos_ytransform() should also invert gl_PointCoord. */
>>>     bool lower_wpos_pntc;
>>>
>>> +       /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
>>> +        * In presence of NaNs, this is correct in GLSL because the behaviour is
>>> +        * undefined. In Vulkan, doing these transformations is incorrect.
>>> +        */
>>> +       bool exact_float_comparisons;
>>> +
>>>     /**
>>>      * Should nir_lower_io() create load_interpolated_input intrinsics?
>>>      *
>>> diff --git a/src/compiler/nir/nir_opt_algebraic.py b/src/compiler/nir/nir_opt_algebraic.py
>>> index f2a7be0c403..3750874407b 100644
>>> --- a/src/compiler/nir/nir_opt_algebraic.py
>>> +++ b/src/compiler/nir/nir_opt_algebraic.py
>>> @@ -154,10 +154,10 @@ optimizations = [
>>>     (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
>>>
>>>     # Comparison simplifications
>>> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
>>> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
>>> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
>>> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
>>> +   (('~inot', ('flt', a, b)), ('fge', a, b), '!options->exact_float_comparisons'),
>>> +   (('~inot', ('fge', a, b)), ('flt', a, b), '!options->exact_float_comparisons'),
>>> +   (('~inot', ('feq', a, b)), ('fne', a, b), '!options->exact_float_comparisons'),
>>> +   (('~inot', ('fne', a, b)), ('feq', a, b), '!options->exact_float_comparisons'),
>>
>>
>> The feq/fne one is actually completely safe.  fne is defined to be !feq even when NaN is considered.
>>
>> --Jasoan
>>
>>>
>>>     (('inot', ('ilt', a, b)), ('ige', a, b)),
>>>     (('inot', ('ult', a, b)), ('uge', a, b)),
>>>     (('inot', ('ige', a, b)), ('ilt', a, b)),
>>> --
>>> 2.19.2
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>