On Sat, Dec 1, 2018 at 3:22 PM Samuel Pitoiset <[email protected]> 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 > > <[email protected] <mailto:[email protected]>> 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 <[email protected] > > <mailto:[email protected]>> > > --- > > 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 > > [email protected] <mailto:[email protected]> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
