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

[Samuel Pitoiset]

I'm not saying this series is the right thing to do. It just fixes two 
test failures in the vkd3d testsuite for RADV. I added a new compiler 
option to not break anything and to only affects RADV. Anyways, it seems 
unclear what the best option is. To sum up, looks like there is 3 ways:

1) set the exact bit for all SPIRV float comparisons ops
2) draft a new extension, not sure if we really need to
3) just remove these optimisations when targeting Vulkan

Opinions are welcome, thanks!

On 11/29/18 4:22 PM, Jason Ekstrand 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".
> 
> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset 
> <samuel.pitoiset at gmail.com <mailto: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
>     <mailto: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 <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>