* [PATCH] ppc/translate: Fix unordered f64/f128 comparisons
@ 2020-11-09 10:21 LemonBoy
2020-11-10 3:24 ` Richard Henderson
0 siblings, 1 reply; 3+ messages in thread
From: LemonBoy @ 2020-11-09 10:21 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
According to the PowerISA v3.1 reference, Table 68 "Actions for xscmpudp
- Part 1: Compare Unordered", whenever one of the two operands is a NaN
the SO bit is set while the other three bits are cleared.
Apply the same change to xscmpuqp.
The respective ordered counterparts are unaffected.
Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 9b8c8b70b6..b07ff66375 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2479,13 +2479,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) || \
float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) { \
vxsnan_flag = true; \
- cc = CRF_SO; \
if (fpscr_ve == 0 && ordered) { \
vxvc_flag = true; \
} \
} else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) || \
float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) { \
- cc = CRF_SO; \
if (ordered) { \
vxvc_flag = true; \
} \
@@ -2497,12 +2495,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
float_invalid_op_vxvc(env, 0, GETPC()); \
} \
\
- if (float64_lt(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
+ switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
+ case float_relation_less: \
cc |= CRF_LT; \
- } else if (!float64_le(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
- cc |= CRF_GT; \
- } else { \
+ break; \
+ case float_relation_equal: \
cc |= CRF_EQ; \
+ break; \
+ case float_relation_greater: \
+ cc |= CRF_GT; \
+ break; \
+ case float_relation_unordered: \
+ cc |= CRF_SO; \
+ break; \
} \
\
env->fpscr &= ~FP_FPCC; \
@@ -2545,12 +2550,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
float_invalid_op_vxvc(env, 0, GETPC()); \
} \
\
- if (float128_lt(xa->f128, xb->f128, &env->fp_status)) { \
+ switch (float128_compare(xa->f128, xb->f128, &env->fp_status)) { \
+ case float_relation_less: \
cc |= CRF_LT; \
- } else if (!float128_le(xa->f128, xb->f128, &env->fp_status)) { \
- cc |= CRF_GT; \
- } else { \
+ break; \
+ case float_relation_equal: \
cc |= CRF_EQ; \
+ break; \
+ case float_relation_greater: \
+ cc |= CRF_GT; \
+ break; \
+ case float_relation_unordered: \
+ cc |= CRF_SO; \
+ break; \
} \
\
env->fpscr &= ~FP_FPCC; \
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ppc/translate: Fix unordered f64/f128 comparisons
2020-11-09 10:21 [PATCH] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
@ 2020-11-10 3:24 ` Richard Henderson
2020-11-10 9:06 ` LemonBoy
0 siblings, 1 reply; 3+ messages in thread
From: Richard Henderson @ 2020-11-10 3:24 UTC (permalink / raw)
To: LemonBoy, qemu-devel; +Cc: qemu-ppc, david
On 11/9/20 2:21 AM, LemonBoy wrote:
> According to the PowerISA v3.1 reference, Table 68 "Actions for xscmpudp
> - Part 1: Compare Unordered", whenever one of the two operands is a NaN
> the SO bit is set while the other three bits are cleared.
>
> Apply the same change to xscmpuqp.
>
> The respective ordered counterparts are unaffected.
>
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
> target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 9b8c8b70b6..b07ff66375 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -2479,13 +2479,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
> if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) || \
> float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) { \
> vxsnan_flag = true; \
> - cc = CRF_SO; \
> if (fpscr_ve == 0 && ordered) { \
> vxvc_flag = true; \
> } \
> } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) || \
> float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) { \
> - cc = CRF_SO; \
> if (ordered) { \
> vxvc_flag = true; \
> } \
> @@ -2497,12 +2495,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
> float_invalid_op_vxvc(env, 0, GETPC()); \
> } \
> \
> - if (float64_lt(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
> + switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
> + case float_relation_less: \
> cc |= CRF_LT; \
> - } else if (!float64_le(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
> - cc |= CRF_GT; \
> - } else { \
> + break; \
> + case float_relation_equal: \
> cc |= CRF_EQ; \
> + break; \
> + case float_relation_greater: \
> + cc |= CRF_GT; \
> + break; \
> + case float_relation_unordered: \
> + cc |= CRF_SO; \
> + break; \
> }
This needs some more cleanup. There's no point in checking for nans first;
wait until you get to float_relation_unordered.
These macros should be made into straight functions.
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ppc/translate: Fix unordered f64/f128 comparisons
2020-11-10 3:24 ` Richard Henderson
@ 2020-11-10 9:06 ` LemonBoy
0 siblings, 0 replies; 3+ messages in thread
From: LemonBoy @ 2020-11-10 9:06 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 10/11/20 04:24, Richard Henderson wrote:
> On 11/9/20 2:21 AM, LemonBoy wrote:
>> According to the PowerISA v3.1 reference, Table 68 "Actions for xscmpudp
>> - Part 1: Compare Unordered", whenever one of the two operands is a NaN
>> the SO bit is set while the other three bits are cleared.
>>
>> Apply the same change to xscmpuqp.
>>
>> The respective ordered counterparts are unaffected.
>>
>> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
>> ---
>> target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
>> index 9b8c8b70b6..b07ff66375 100644
>> --- a/target/ppc/fpu_helper.c
>> +++ b/target/ppc/fpu_helper.c
>> @@ -2479,13 +2479,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
>> if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) || \
>> float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) { \
>> vxsnan_flag = true; \
>> - cc = CRF_SO; \
>> if (fpscr_ve == 0 && ordered) { \
>> vxvc_flag = true; \
>> } \
>> } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) || \
>> float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) { \
>> - cc = CRF_SO; \
>> if (ordered) { \
>> vxvc_flag = true; \
>> } \
>> @@ -2497,12 +2495,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
>> float_invalid_op_vxvc(env, 0, GETPC()); \
>> } \
>> \
>> - if (float64_lt(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
>> + switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
>> + case float_relation_less: \
>> cc |= CRF_LT; \
>> - } else if (!float64_le(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
>> - cc |= CRF_GT; \
>> - } else { \
>> + break; \
>> + case float_relation_equal: \
>> cc |= CRF_EQ; \
>> + break; \
>> + case float_relation_greater: \
>> + cc |= CRF_GT; \
>> + break; \
>> + case float_relation_unordered: \
>> + cc |= CRF_SO; \
>> + break; \
>> }
>
> This needs some more cleanup. There's no point in checking for nans first;
> wait until you get to float_relation_unordered.
>
> These macros should be made into straight functions.
>
Will do, I tried to keep the amount of changed lines as small as possible
but I'm happy to nuke that macro: aligning the trailing slashes took more
time than finding and fixing the bug :)
>
> r~
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-10 9:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-09 10:21 [PATCH] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
2020-11-10 3:24 ` Richard Henderson
2020-11-10 9:06 ` LemonBoy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.