| Message ID | 1302645571-20500-18-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 12, 2011 at 11:59:29PM +0200, Aurelien Jarno wrote: > Given that float32_*() functions are IEEE754 compliant, the efscmp*() > functions are correctly implemented, while efstst*() are not. This > patch reverse the implementation of this two groups of functions and > fix the comments. It also use float32_eq() instead of float32_eq_quiet() > as qNaNs should not be ignored. Thanks for addressing this; the E500 emulation in QEMU is more like how we wish the hardware acted, rather than how it actually acts. :) It's late here, but I think this change: > -static inline uint32_t efscmplt(uint32_t op1, uint32_t op2) > +static inline uint32_t efststlt(uint32_t op1, uint32_t op2) > { > - /* XXX: TODO: test special values (NaN, infinites, ...) */ > + /* XXX: TODO: ignore special values (NaN, infinites, ...) */ > return efststlt(op1, op2); > } is not correct, as you've just turned this into an infinite (inlined!) loop. You'd want to change the efststlt call into an efscmplt call. Similarly for efstst{gt,eq}. -Nathan
On Tue, Apr 12, 2011 at 07:40:33PM -0700, Nathan Froyd wrote: > On Tue, Apr 12, 2011 at 11:59:29PM +0200, Aurelien Jarno wrote: > > Given that float32_*() functions are IEEE754 compliant, the efscmp*() > > functions are correctly implemented, while efstst*() are not. This > > patch reverse the implementation of this two groups of functions and > > fix the comments. It also use float32_eq() instead of float32_eq_quiet() > > as qNaNs should not be ignored. > > Thanks for addressing this; the E500 emulation in QEMU is more like how > we wish the hardware acted, rather than how it actually acts. :) > > It's late here, but I think this change: > > > -static inline uint32_t efscmplt(uint32_t op1, uint32_t op2) > > +static inline uint32_t efststlt(uint32_t op1, uint32_t op2) > > { > > - /* XXX: TODO: test special values (NaN, infinites, ...) */ > > + /* XXX: TODO: ignore special values (NaN, infinites, ...) */ > > return efststlt(op1, op2); > > } > > is not correct, as you've just turned this into an infinite (inlined!) > loop. You'd want to change the efststlt call into an efscmplt call. > Similarly for efstst{gt,eq}. > Thanks for catching that, I'll fix that in the next version.
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c index f645f57..696958a 100644 --- a/target-ppc/op_helper.c +++ b/target-ppc/op_helper.c @@ -3331,7 +3331,7 @@ HELPER_SPE_VECTOR_ARITH(fsmul); HELPER_SPE_VECTOR_ARITH(fsdiv); /* Single-precision floating-point comparisons */ -static inline uint32_t efststlt(uint32_t op1, uint32_t op2) +static inline uint32_t efscmplt(uint32_t op1, uint32_t op2) { CPU_FloatU u1, u2; u1.l = op1; @@ -3339,7 +3339,7 @@ static inline uint32_t efststlt(uint32_t op1, uint32_t op2) return float32_lt(u1.f, u2.f, &env->vec_status) ? 4 : 0; } -static inline uint32_t efststgt(uint32_t op1, uint32_t op2) +static inline uint32_t efscmpgt(uint32_t op1, uint32_t op2) { CPU_FloatU u1, u2; u1.l = op1; @@ -3347,29 +3347,29 @@ static inline uint32_t efststgt(uint32_t op1, uint32_t op2) return float32_le(u1.f, u2.f, &env->vec_status) ? 0 : 4; } -static inline uint32_t efststeq(uint32_t op1, uint32_t op2) +static inline uint32_t efscmpeq(uint32_t op1, uint32_t op2) { CPU_FloatU u1, u2; u1.l = op1; u2.l = op2; - return float32_eq_quiet(u1.f, u2.f, &env->vec_status) ? 4 : 0; + return float32_eq(u1.f, u2.f, &env->vec_status) ? 4 : 0; } -static inline uint32_t efscmplt(uint32_t op1, uint32_t op2) +static inline uint32_t efststlt(uint32_t op1, uint32_t op2) { - /* XXX: TODO: test special values (NaN, infinites, ...) */ + /* XXX: TODO: ignore special values (NaN, infinites, ...) */ return efststlt(op1, op2); } -static inline uint32_t efscmpgt(uint32_t op1, uint32_t op2) +static inline uint32_t efststgt(uint32_t op1, uint32_t op2) { - /* XXX: TODO: test special values (NaN, infinites, ...) */ + /* XXX: TODO: ignore special values (NaN, infinites, ...) */ return efststgt(op1, op2); } -static inline uint32_t efscmpeq(uint32_t op1, uint32_t op2) +static inline uint32_t efststeq(uint32_t op1, uint32_t op2) { - /* XXX: TODO: test special values (NaN, infinites, ...) */ + /* XXX: TODO: ignore special values (NaN, infinites, ...) */ return efststeq(op1, op2); }
efstst*() functions are fast SPE funtions which do not take into account special values (infinites, NaN, etc.), while efscmp*() functions are IEEE754 compliant. Given that float32_*() functions are IEEE754 compliant, the efscmp*() functions are correctly implemented, while efstst*() are not. This patch reverse the implementation of this two groups of functions and fix the comments. It also use float32_eq() instead of float32_eq_quiet() as qNaNs should not be ignored. Cc: Alexander Graf <agraf@suse.de> Cc: Nathan Froyd <froydnj@codesourcery.com> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> (cherry picked from commit 9f4055a7d1426c6a1f6b11725e3458ada4cbc5eb) --- target-ppc/op_helper.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-)