On Fri, Nov 30, 2018 at 3:37 PM Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl> wrote:
> On Fri, Nov 30, 2018 at 10:29 PM Jason Ekstrand <ja...@jlekstrand.net> > wrote: > > > > On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick <i...@freedesktop.org> > wrote: > >> > >> On 11/29/2018 07: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 > >> > >> 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. > > > > > > 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. > > What do you mean an extension? AFAIU the concern is that Vulkan SPIR-V > is more restrictive than GLSL here, and disallows these optimization > right? That makes a strong case that we should remove these rules for > at least Vulkan. If that means writing a CTS test, maybe we should do > just that? > I know there were some discussions around this. I don't remember what the outcome was. It's possible that it really was "Vulkan is just more restrictive" but I don't remember. I know there are CTS tests for these opcodes and, last I checked, we're passing the CTS so maybe those tests aren't mustpass? I'd have to go digging. --Jason > > > >> > >> >> 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 >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev