* [Qemu-devel] [PATCH] target-sparc: fix fcmp{s, d, q} instructions wrt exception
@ 2012-09-07 15:13 Aurelien Jarno
2012-09-08 9:08 ` Blue Swirl
0 siblings, 1 reply; 2+ messages in thread
From: Aurelien Jarno @ 2012-09-07 15:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, Aurelien Jarno
fcmp{s,d,q} instructions are supposed to ignore quiet NaN (contrary to
the fcmpe{s,d,q} instructions), but the current code is wrongly setting
the NV exception in that case. Moreover the current code is duplicated:
first the arguments are checked for NaN to generate an exception, and
later in case the comparison is unordered (which can only happens if one
of the argument is a NaN), the same check is done to generate an
exception.
Fix that by calling clear_float_exceptions() followed by
check_ieee_exceptions() as for the other floating point instructions.
Use the _compare_quiet functions for fcmp{s,d,q} and the _compare ones
for fcmpe{s,d,q}. Simplify the flag setting by not clearing a flag that
is set the line just below.
This fix allows the math glibc testsuite to pass.
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
target-sparc/fop_helper.c | 67 ++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 40 deletions(-)
diff --git a/target-sparc/fop_helper.c b/target-sparc/fop_helper.c
index 9c64ef8..f4b62a5 100644
--- a/target-sparc/fop_helper.c
+++ b/target-sparc/fop_helper.c
@@ -334,34 +334,28 @@ void helper_fsqrtq(CPUSPARCState *env)
}
#define GEN_FCMP(name, size, reg1, reg2, FS, E) \
- void glue(helper_, name) (CPUSPARCState *env) \
+ void glue(helper_, name) (CPUSPARCState *env) \
{ \
- env->fsr &= FSR_FTT_NMASK; \
- if (E && (glue(size, _is_any_nan)(reg1) || \
- glue(size, _is_any_nan)(reg2)) && \
- (env->fsr & FSR_NVM)) { \
- env->fsr |= FSR_NVC; \
- env->fsr |= FSR_FTT_IEEE_EXCP; \
- helper_raise_exception(env, TT_FP_EXCP); \
+ int ret; \
+ clear_float_exceptions(env); \
+ if (E) { \
+ ret = glue(size, _compare)(reg1, reg2, &env->fp_status); \
+ } else { \
+ ret = glue(size, _compare_quiet)(reg1, reg2, \
+ &env->fp_status); \
} \
- switch (glue(size, _compare) (reg1, reg2, &env->fp_status)) { \
+ check_ieee_exceptions(env); \
+ switch (ret) { \
case float_relation_unordered: \
- if ((env->fsr & FSR_NVM)) { \
- env->fsr |= FSR_NVC; \
- env->fsr |= FSR_FTT_IEEE_EXCP; \
- helper_raise_exception(env, TT_FP_EXCP); \
- } else { \
- env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
- env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
- env->fsr |= FSR_NVA; \
- } \
+ env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
+ env->fsr |= FSR_NVA; \
break; \
case float_relation_less: \
- env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
+ env->fsr &= ~(FSR_FCC1) << FS; \
env->fsr |= FSR_FCC0 << FS; \
break; \
case float_relation_greater: \
- env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
+ env->fsr &= ~(FSR_FCC0) << FS; \
env->fsr |= FSR_FCC1 << FS; \
break; \
default: \
@@ -370,34 +364,27 @@ void helper_fsqrtq(CPUSPARCState *env)
} \
}
#define GEN_FCMP_T(name, size, FS, E) \
- void glue(helper_, name)(CPUSPARCState *env, size src1, size src2) \
+ void glue(helper_, name)(CPUSPARCState *env, size src1, size src2) \
{ \
- env->fsr &= FSR_FTT_NMASK; \
- if (E && (glue(size, _is_any_nan)(src1) || \
- glue(size, _is_any_nan)(src2)) && \
- (env->fsr & FSR_NVM)) { \
- env->fsr |= FSR_NVC; \
- env->fsr |= FSR_FTT_IEEE_EXCP; \
- helper_raise_exception(env, TT_FP_EXCP); \
+ int ret; \
+ clear_float_exceptions(env); \
+ if (E) { \
+ ret = glue(size, _compare)(src1, src2, &env->fp_status); \
+ } else { \
+ ret = glue(size, _compare_quiet)(src1, src2, \
+ &env->fp_status); \
} \
- switch (glue(size, _compare) (src1, src2, &env->fp_status)) { \
+ check_ieee_exceptions(env); \
+ switch (ret) { \
case float_relation_unordered: \
- if ((env->fsr & FSR_NVM)) { \
- env->fsr |= FSR_NVC; \
- env->fsr |= FSR_FTT_IEEE_EXCP; \
- helper_raise_exception(env, TT_FP_EXCP); \
- } else { \
- env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
- env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
- env->fsr |= FSR_NVA; \
- } \
+ env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
break; \
case float_relation_less: \
- env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
+ env->fsr &= ~(FSR_FCC1 << FS); \
env->fsr |= FSR_FCC0 << FS; \
break; \
case float_relation_greater: \
- env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
+ env->fsr &= ~(FSR_FCC0 << FS); \
env->fsr |= FSR_FCC1 << FS; \
break; \
default: \
--
1.7.10.4
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [Qemu-devel] [PATCH] target-sparc: fix fcmp{s, d, q} instructions wrt exception
2012-09-07 15:13 [Qemu-devel] [PATCH] target-sparc: fix fcmp{s, d, q} instructions wrt exception Aurelien Jarno
@ 2012-09-08 9:08 ` Blue Swirl
0 siblings, 0 replies; 2+ messages in thread
From: Blue Swirl @ 2012-09-08 9:08 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
Thanks, applied.
On Fri, Sep 7, 2012 at 3:13 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> fcmp{s,d,q} instructions are supposed to ignore quiet NaN (contrary to
> the fcmpe{s,d,q} instructions), but the current code is wrongly setting
> the NV exception in that case. Moreover the current code is duplicated:
> first the arguments are checked for NaN to generate an exception, and
> later in case the comparison is unordered (which can only happens if one
> of the argument is a NaN), the same check is done to generate an
> exception.
>
> Fix that by calling clear_float_exceptions() followed by
> check_ieee_exceptions() as for the other floating point instructions.
> Use the _compare_quiet functions for fcmp{s,d,q} and the _compare ones
> for fcmpe{s,d,q}. Simplify the flag setting by not clearing a flag that
> is set the line just below.
>
> This fix allows the math glibc testsuite to pass.
>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-sparc/fop_helper.c | 67 ++++++++++++++++++---------------------------
> 1 file changed, 27 insertions(+), 40 deletions(-)
>
> diff --git a/target-sparc/fop_helper.c b/target-sparc/fop_helper.c
> index 9c64ef8..f4b62a5 100644
> --- a/target-sparc/fop_helper.c
> +++ b/target-sparc/fop_helper.c
> @@ -334,34 +334,28 @@ void helper_fsqrtq(CPUSPARCState *env)
> }
>
> #define GEN_FCMP(name, size, reg1, reg2, FS, E) \
> - void glue(helper_, name) (CPUSPARCState *env) \
> + void glue(helper_, name) (CPUSPARCState *env) \
> { \
> - env->fsr &= FSR_FTT_NMASK; \
> - if (E && (glue(size, _is_any_nan)(reg1) || \
> - glue(size, _is_any_nan)(reg2)) && \
> - (env->fsr & FSR_NVM)) { \
> - env->fsr |= FSR_NVC; \
> - env->fsr |= FSR_FTT_IEEE_EXCP; \
> - helper_raise_exception(env, TT_FP_EXCP); \
> + int ret; \
> + clear_float_exceptions(env); \
> + if (E) { \
> + ret = glue(size, _compare)(reg1, reg2, &env->fp_status); \
> + } else { \
> + ret = glue(size, _compare_quiet)(reg1, reg2, \
> + &env->fp_status); \
> } \
> - switch (glue(size, _compare) (reg1, reg2, &env->fp_status)) { \
> + check_ieee_exceptions(env); \
> + switch (ret) { \
> case float_relation_unordered: \
> - if ((env->fsr & FSR_NVM)) { \
> - env->fsr |= FSR_NVC; \
> - env->fsr |= FSR_FTT_IEEE_EXCP; \
> - helper_raise_exception(env, TT_FP_EXCP); \
> - } else { \
> - env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
> - env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
> - env->fsr |= FSR_NVA; \
> - } \
> + env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
> + env->fsr |= FSR_NVA; \
> break; \
> case float_relation_less: \
> - env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
> + env->fsr &= ~(FSR_FCC1) << FS; \
> env->fsr |= FSR_FCC0 << FS; \
> break; \
> case float_relation_greater: \
> - env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
> + env->fsr &= ~(FSR_FCC0) << FS; \
> env->fsr |= FSR_FCC1 << FS; \
> break; \
> default: \
> @@ -370,34 +364,27 @@ void helper_fsqrtq(CPUSPARCState *env)
> } \
> }
> #define GEN_FCMP_T(name, size, FS, E) \
> - void glue(helper_, name)(CPUSPARCState *env, size src1, size src2) \
> + void glue(helper_, name)(CPUSPARCState *env, size src1, size src2) \
> { \
> - env->fsr &= FSR_FTT_NMASK; \
> - if (E && (glue(size, _is_any_nan)(src1) || \
> - glue(size, _is_any_nan)(src2)) && \
> - (env->fsr & FSR_NVM)) { \
> - env->fsr |= FSR_NVC; \
> - env->fsr |= FSR_FTT_IEEE_EXCP; \
> - helper_raise_exception(env, TT_FP_EXCP); \
> + int ret; \
> + clear_float_exceptions(env); \
> + if (E) { \
> + ret = glue(size, _compare)(src1, src2, &env->fp_status); \
> + } else { \
> + ret = glue(size, _compare_quiet)(src1, src2, \
> + &env->fp_status); \
> } \
> - switch (glue(size, _compare) (src1, src2, &env->fp_status)) { \
> + check_ieee_exceptions(env); \
> + switch (ret) { \
> case float_relation_unordered: \
> - if ((env->fsr & FSR_NVM)) { \
> - env->fsr |= FSR_NVC; \
> - env->fsr |= FSR_FTT_IEEE_EXCP; \
> - helper_raise_exception(env, TT_FP_EXCP); \
> - } else { \
> - env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
> - env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
> - env->fsr |= FSR_NVA; \
> - } \
> + env->fsr |= (FSR_FCC1 | FSR_FCC0) << FS; \
> break; \
> case float_relation_less: \
> - env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
> + env->fsr &= ~(FSR_FCC1 << FS); \
> env->fsr |= FSR_FCC0 << FS; \
> break; \
> case float_relation_greater: \
> - env->fsr &= ~((FSR_FCC1 | FSR_FCC0) << FS); \
> + env->fsr &= ~(FSR_FCC0 << FS); \
> env->fsr |= FSR_FCC1 << FS; \
> break; \
> default: \
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-09-08 9:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-07 15:13 [Qemu-devel] [PATCH] target-sparc: fix fcmp{s, d, q} instructions wrt exception Aurelien Jarno
2012-09-08 9:08 ` Blue Swirl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).