<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Feb 26, 2018 at 2:21 PM, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 02/23/2018 05:14 PM, Jason Ekstrand wrote:<br>
> On Fri, Feb 23, 2018 at 3:55 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a><br>
</span><span class="">> <mailto:<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>>> wrote:<br>
><br>
> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a><br>
</span>> <mailto:<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.<wbr>com</a>>><br>
<span class="">><br>
> shader-db results:<br>
><br>
> Haswell, Broadwell, and Skylake had similar results. (Skylake shown)<br>
> total instructions in shared programs: 14514817 -> 14514808 (<.01%)<br>
> instructions in affected programs: 229 -> 220 (-3.93%)<br>
> helped: 3<br>
> HURT: 0<br>
> helped stats (abs) min: 1 max: 4 x̄: 3.00 x̃: 4<br>
> helped stats (rel) min: 2.86% max: 4.12% x̄: 3.70% x̃: 4.12%<br>
><br>
> total cycles in shared programs: 533145211 -> 533144939 (<.01%)<br>
> cycles in affected programs: 37268 -> 36996 (-0.73%)<br>
> helped: 8<br>
> HURT: 0<br>
> helped stats (abs) min: 2 max: 134 x̄: 34.00 x̃: 2<br>
> helped stats (rel) min: 0.02% max: 14.22% x̄: 3.53% x̃: 0.05%<br>
><br>
> Sandy Bridge and Ivy Bridge had similar results. (Ivy Bridge shown)<br>
> total cycles in shared programs: 257618409 -> 257618403 (<.01%)<br>
> cycles in affected programs: 12582 -> 12576 (-0.05%)<br>
> helped: 3<br>
> HURT: 0<br>
> helped stats (abs) min: 2 max: 2 x̄: 2.00 x̃: 2<br>
> helped stats (rel) min: 0.05% max: 0.05% x̄: 0.05% x̃: 0.05%<br>
><br>
> No changes on Iron Lake or GM45.<br>
><br>
> Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a><br>
</span>> <mailto:<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.<wbr>com</a>>><br>
<span class="">> ---<br>
> src/compiler/nir/nir_opt_<wbr>algebraic.py | 2 ++<br>
> 1 file changed, 2 insertions(+)<br>
><br>
> diff --git a/src/compiler/nir/nir_opt_<wbr>algebraic.py<br>
> b/src/compiler/nir/nir_opt_<wbr>algebraic.py<br>
> index d40d59b..f5f9e94 100644<br>
> --- a/src/compiler/nir/nir_opt_<wbr>algebraic.py<br>
> +++ b/src/compiler/nir/nir_opt_<wbr>algebraic.py<br>
> @@ -170,6 +170,8 @@ optimizations = [<br>
> (('fge', ('fneg', ('fabs', a)), 0.0), ('feq', a, 0.0)),<br>
> (('bcsel', ('flt', b, a), b, a), ('fmin', a, b)),<br>
> (('bcsel', ('flt', a, b), b, a), ('fmax', a, b)),<br>
> + (('bcsel', ('fge', b, a), a, b), ('fmin', a, b)),<br>
> + (('bcsel', ('fge', a, b), a, b), ('fmax', a, b)),<br>
><br>
><br>
> Please flag as inexact. As per the stupid GLSL definition, these are<br>
> not the same as fmin/fmax when you throw in a NaN.<br>
<br>
</span>I'm having some trouble rectifying this with the existing<br>
transformations and the Intel hardware implementation.<br></blockquote><div><br></div><div>Me too. :-) Really, I think D3D10 spec'd the right thing. With GL, it looks more like a case of someone wrote the obvious thing down when NaN wasn't a thing and it got extended to NaN without much thought. How would you feel about just defining the NIR
fmin/fmax to have the D3D10 behavior and telling people that they can
make new opcodes if they want something different? At the very least,
that would make it constant-fold consistently for us. It also sounds like it's a correct transformation as per the spec comment below.<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
GLSL spec says min(x, y) "Returns y if y < x; otherwise it returns x."<br>
>From that I infer min(x, NaN) == x, and min(NaN, y) == NaN. The<br>
expression ('bcsel', ('flt', b, a), b, a) has the same behavior.<br>
<br>
I think if I rewrite the fmin transform as (swapping the argument order)<br>
<br>
(('bcsel', ('fge', a, b), b, a), ('fmin', a, b)),<br>
<br>
it should be at least as valid for as the existing transforms. A<br>
similar modification should work for fmax.<br></blockquote><div><br></div><div>Yes, it should.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The Intel SEL instruction which says that with the .L or .GE modifier,<br>
if one argument is NaN, the other value is always returned. This means<br>
that min(NaN, y) will be y.<br>
<br>
This is valid for min and max because section 4.7.1 (Range and<br>
Precision) says:<br>
<br>
Operations and built-in functions that operate on a NaN are not<br>
required to return a NaN as the result.<br></blockquote><div><br></div><div>It's good to know that the DX behavior is considered a valid transformation. I didn't know exactly what the rules were there.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't think returning non-NaN for ('bcsel', ('flt', b, NaN), b, NaN)<br>
is valid, so I think the existing transformations should also be marked<br>
inexact for platforms that implement the "never NaN" behavior for fmin<br>
or fmax.<span class=""><br></span></blockquote><div><br></div><div>I think I agree. I think you could probably argue the point and I doubt real apps would care but... Probably best to flag it as inexact and move on with life. If you're using precise and open-coding min/max in some horifically stupid way, you're doing it wrong.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> (('bcsel', ('inot', a), b, c), ('bcsel', a, c, b)),<br>
> (('bcsel', a, ('bcsel', a, b, c), d), ('bcsel', a, b, d)),<br>
> (('bcsel', a, True, 'b@bool'), ('ior', a, b)),<br>
> --<br>
> 2.9.5<br>
><br>
> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
</span>> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.<wbr>freedesktop.org</a>><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
> <<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.<wbr>org/mailman/listinfo/mesa-dev</a>><br>
</blockquote></div><br></div></div>