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?
>
>>
>> >> 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
