On 12/1/18 3:36 PM, Connor Abbott wrote:
On Sat, Dec 1, 2018 at 3:22 PM Samuel Pitoiset
<samuel.pitoi...@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.

I agree, it's not the first time we have problems with that and it would be very nice to fix it for real. vkd3d is just one example.



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.pitoi...@gmail.com <mailto: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
     <mailto: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 <mailto: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