On Thu, Nov 29, 2018, at 7:47 AM, Connor Abbott wrote: > 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.
FYI here are the shader-db results (SKL) from removing them: total instructions in shared programs: 12858124 -> 12889104 (0.24%) instructions in affected programs: 1687380 -> 1718360 (1.84%) helped: 2 HURT: 7073 total cycles in shared programs: 317838109 -> 318266406 (0.13%) cycles in affected programs: 62285268 -> 62713565 (0.69%) helped: 1017 HURT: 6552 total loops in shared programs: 3808 -> 3808 (0.00%) loops in affected programs: 0 -> 0 helped: 0 HURT: 0 total spills in shared programs: 6793 -> 6785 (-0.12%) spills in affected programs: 166 -> 158 (-4.82%) helped: 2 HURT: 0 total fills in shared programs: 9561 -> 9541 (-0.21%) fills in affected programs: 852 -> 832 (-2.35%) helped: 2 HURT: 3 LOST: 0 GAINED: 1 > > > > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev