[Mesa-dev] IROUND() issue

[Brian Paul]

On 05/18/2012 10:11 AM, Jose Fonseca wrote:
>
>
> ----- Original Message -----
>>
>> A while back I noticed that the piglit roundmode-pixelstore and
>> roundmode-getinteger tests pass on my 64-bit Fedora system but fail
>> on
>> a 32-bit Ubuntu system.  Both glGetIntegerv() and glPixelStoref()
>>   use
>> the IROUND() function to convert floats to ints.
>>
>> The implementation if IROUND() that uses the x86 fistp instruction is
>> protected with:
>>
>> #if defined(USE_X86_ASM)&&  defined(__GNUC__)&&  defined(__i386__)
>>
>> but that evaluates to 0 on x86-64 (neither USE_X86_ASM nor __i386__
>> are defined) so we use the C fallback:
>>
>> #define IROUND(f)  ((int) (((f)>= 0.0F) ? ((f) + 0.5F) : ((f) -
>> 0.5F)))
>>
>> The C version of IROUND() does what we want for the piglit tests but
>> not the x86 version.  I think the default x86 rounding mode is
>> FE_UPWARD so that explains the failures.
>>
>>
>> So I think I'd like to do the following:
>>
>> 1. Enable the x86 fistp-based functions in imports.h for x86-64.
>
> It's illegal/inneficient to use x87 on x86-64. We should use the appropriate SSE intrisinsic instead.
>
>> 2. Rename IROUND() to IROUND_FAST() and define it as float->int
>> conversion by whatever method is fastest.
>>
>> 3. Define IROUND() as round to nearest int.  For the x86 fistp
>> implementation this would involve setting/restoring the rounding
>> mode.
>>
>> 4. Do an inspection of current IROUND() calls and convert some to
>> IROUND_FAST() where we can get away with it.
>
> It sounds a sensible plan, but I wonder if the multiple iround versions are really worth it: SSE2 is guaranteed in x64, so we could just start requiring SSE2 on x86, and only use the C for other platforms.

The SSE2 plan sounds good.  Though, I'd need some help with the 
implementation.  My x86 asm skills are pretty weak.

-Brian