On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand <ja...@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
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.

>
> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset <samuel.pitoi...@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.pitoi...@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@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to