x86: FPU_MAX, FPU_MIN incorrect

Bug #889053 reported by Daniil Troshkov
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Fix Released
Undecided
Unassigned

Bug Description

Dear All,

Bug was found in qemu.git.
Now (0.15, 1.0) all fpu is softfpu.
See target-i386/ops_sse.h:
#define FPU_MIN(size, a, b) (a) < (b) ? (a) : (b)
#define FPU_MAX(size, a, b) (a) > (b) ? (a) : (b)
It is incorrect now, becouse float64 (or 32) is (typedef) uint64_t (or 32).
And if we have signed operands we get error...

There is a test with this error (spec shinx3 test data, results diffs on machine and qemu (linux)) and fixed patch. See attach.

Daniil.

Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :
Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :
Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :

misprint:
spec sphinx3 test data

Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :
Revision history for this message
Peter Maydell (pmaydell) wrote :

The attached patch is incorrect (using the softfloat _min/_max functions will give wrong answers for some special cases). The correct macros are
#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)

(see recent discussion on the qemu-devel list).

Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :

Hello!
Can I commit this patch (in development branch), and close this bug...
Or you must do it?

Revision history for this message
Peter Maydell (pmaydell) wrote :

Your patch is broken, as I said before.

Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :

Yes, but you patch is correct...

Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :

May be commit you patch

Revision history for this message
Daniil Troshkov (troshkovdanil) wrote :

I say about
#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) ? (b) : (a)

Revision history for this message
Thomas Huth (th-huth) wrote :

The patch mentioned in comment #5 has been included here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=a4d1f142542935b90d2e

Changed in qemu:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.