<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 28, 2014 at 8:34 AM, Roland Scheidegger <span dir="ltr"><<a href="mailto:sroland@vmware.com" target="_blank">sroland@vmware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Looking all good, though it got me thinking about these numerous<br>
float-to-int rounding functions in mesa in various places a bit more.<br>
There is, for instance, a _mesa_round_to_even() function, which claims<br>
that the c99 lrintf() and friends functions can't be used because the<br>
environment might use a different rounding mode. This is of course true<br>
though I wonder why it would matter there and not here for instance,<br>
since of course cvtss2si is affected by that as well.<br></blockquote><div><br></div><div>Yes, it does matter for cvtss2si.  I briefly considered replacing F_TO_I with lrint and just trusting in the standard library to do it.  However, that turns out to have really bad performance (on my system) if you don't enable -ffast-math (which we don't by default).  The reason why using cvtss2si is ok is that we only use F_TO_I for things where the GL spec allows us to be sloppy on the rounding.  In particular, we use it for texture format conversion and for things like swrast where we care more about fast than perfect.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, I'm wondering about the IROUND fallback. Richard Sandiford<br>
discovered a problem in llvmpipe (using lp_build_iround()) which was<br>
using the same method (that is, (int)(val + 0.5) for unsigned numbers,<br>
similar but more complex for signed numbers). This works ok for nearly<br>
all numbers (though it is definitely not round to nearest even) except<br>
numbers between [-]2^23 and [-]2^24-1, in which case it will always<br>
return the next higher even number for odd numbers. So<br>
IROUND((float)val) != val even for numbers which can be represented as<br>
floats exactly. I'd guess though mesa probably doesn't use IROUND for<br>
cases where this would matter (most likely some conversion of z24<br>
numbers), worst case there would probably not just be the inaccuracy but<br>
if you'd have clamped to z24 max (as float, 2^24-1) then done the IROUND<br>
you'd get back 2^24. And FWIW some gallium util math function seem to<br>
have this problem as well, though again I don't know if it would matter<br>
(the gallium util code will only use them if c99 isn't available). </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">But anyway I guess that's slightly off-topic...<br>
</blockquote><div><br></div><div>That's interesting.  I'll give that some thought, but at that point you're right at the boundary of floating-point precision and things get interesting.  I'll think about it and if I come up with a better way to do it, I'll send a patch.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Roland<br>
<br>
<br>
Am 23.07.2014 05:15, schrieb Jason Ekstrand:<br>
<div class="HOEnZb"><div class="h5">> According to a quick micro-benchmark, this new version is 20% faster on my<br>
> Haswell laptop.<br>
><br>
> v2: Removed the XXX note about x86_64 from the comment<br>
><br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>><br>
> ---<br>
>  src/mesa/main/imports.h | 5 ++++-<br>
>  1 file changed, 4 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/mesa/main/imports.h b/src/mesa/main/imports.h<br>
> index af780b2..c8ae7f2 100644<br>
> --- a/src/mesa/main/imports.h<br>
> +++ b/src/mesa/main/imports.h<br>
> @@ -277,7 +277,6 @@ static inline int IROUND_POS(float f)<br>
><br>
>  /**<br>
>   * Convert float to int using a fast method.  The rounding mode may vary.<br>
> - * XXX We could use an x86-64/SSE2 version here.<br>
>   */<br>
>  static inline int F_TO_I(float f)<br>
>  {<br>
> @@ -285,6 +284,10 @@ static inline int F_TO_I(float f)<br>
>     int r;<br>
>     __asm__ ("fistpl %0" : "=m" (r) : "t" (f) : "st");<br>
>     return r;<br>
> +#elif defined(USE_X86_64_ASM) && defined(__GNUC__)<br>
> +   int r;<br>
> +   __asm__ ("cvtss2si %1, %0" : "=r" (r) : "xm" (f));<br>
> +   return r;<br>
>  #elif defined(USE_X86_ASM) && defined(_MSC_VER)<br>
>     int r;<br>
>     _asm {<br>
><br>
<br>
</div></div></blockquote></div><br></div></div>