<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 11/29/2018 07:47 AM, Connor Abbott wrote: > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> 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 If you're that paranoid about it, why not just mark the operations are precise? That's literally why it exists. > 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. What I hear you saying is, "The behavior isn't defined." Unless you can point to a CTS test or an application that has incorrect behavior, I'm going to oppose removing this pretty strongly. *Every* GLSL compiler does this. </blockquote><div> </div><div>The test case came from VKD3D which does D3D12 on Vulkan. Someone (Samuel, maybe?) was going to ask around and see if we can figure out what D3D12's rules are. It's possible that it requires IEEE or something close. If that's the case, as I said to Samuel on IRC, we're probably looking at an extension. I don't think we want a flag like this that's set per-API. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> >> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset <<a href="mailto:samuel.pitoiset@gmail.com" target="_blank">samuel.pitoiset@gmail.com</a>> 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 <<a href="mailto:samuel.pitoiset@gmail.com" target="_blank">samuel.pitoiset@gmail.com</a>> >>> --- >>> 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 >>> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> >>> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >> >> _______________________________________________ >> mesa-dev mailing list >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > </blockquote></div></div>