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

[Connor Abbott]

On Sat, Dec 1, 2018 at 3:22 PM Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
> 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!

Well, I was just pointing out that this has bit us in the past, and
will probably bite us in the future if we don't fix it once and for
all, so it's about more than the vkd3d testsuite. We're just getting
lucky since right now the optimizations only trigger and mess things
up with this testsuite. After thinking about it, I think it's best if
we do another option 4, which is to remove the optimizations in
question and always do the right thing even for GLSL.

>
> 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
> >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev