Re: [Qemu-devel] [PATCH] target-i386: Fix regression with maxsd SSE2 instruction

[Peter Maydell]

On 8 November 2011 14:45, Jason Wessel <address@hidden> wrote:
> On 11/08/2011 08:40 AM, Peter Maydell wrote:
>> On 8 November 2011 14:22, Jason Wessel <address@hidden> wrote:
>>> +#define FPU_MIN(size, a, b) float ## size ## _lt(a, b, &env->sse_status) ? 
>>> (a) : (b)
>>> +#define FPU_MAX(size, a, b) float ## size ## _lt(b, a, &env->sse_status) ? 
>>> (a) : (b)
>> This will give the wrong answers for special cases involving +0, -0
>> and NaNs. Check the intel architecture manual which says how these
>> should work.
>>
>> (You can't use float*_min() and float*_max() either, as those have
>> sane semantics and you need to implement the Intel ones here.)
>
> Anyone out there know how to fix this the right way?

Having mused about it a bit, I think that actually the macros
there do return the right answers for the special cases :-)

If (a,b) are +0,-0 in some order, then the _lt comparison will
treat them as equal and return 0, so we return b, as required.
If either of (a,b) are NaNs then the _lt comparison will raise
InvalidOp and return 0, so we return b.

That's a bit subtle, so I think it probably deserves a comment:
/* Note that the choice of comparison op here is important to get the
 * special cases right: for min and max Intel specifies that (-0,0),
 * (NaN, anything) and (anything, NaN) return the second argument.
 */

Sorry for the false alarm.

-- PMM