Re: [Qemu-devel] [PATCH 1/3] target/ppc: fmadd check for excp independently

2017-03-03 Thread Richard Henderson

On 03/03/2017 05:58 PM, Nikunj A Dadhania wrote:

Current order of checking does not confirm with the spec
(ISA 3.0: MultiplyAddDP page-469). Change the order and make them
independent of each other.

For example: a = infinity, b = zero, c = SNaN, this should set both
VXIMZ and VXNAN

Signed-off-by: Nikunj A Dadhania 
---
 target/ppc/fpu_helper.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 0535ad0..a547f58 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -747,17 +747,21 @@ static void float64_maddsub_update_excp(CPUPPCState *env, 
float64 arg1,
 float64 arg2, float64 arg3,
 unsigned int madd_flags)
 {
+if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
+ float64_is_signaling_nan(arg2, &env->fp_status) ||
+ float64_is_signaling_nan(arg3, &env->fp_status))) {
+/* sNaN operation */
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
+}
+
 if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
  (float64_is_zero(arg1) && float64_is_infinity(arg2 {
 /* Multiplication of zero by infinity */
-arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
-} else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
-float64_is_signaling_nan(arg2, &env->fp_status) ||
-float64_is_signaling_nan(arg3, &env->fp_status))) {
-/* sNaN operation */
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
-} else if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
-   float64_is_infinity(arg3)) {
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
+}
+


While these two bits should be both be set if the inputs demand, it won't 
happen if the exception enable bit is set, since the first call will longjmp. 
I'm not sure what to do about that; perhaps just ignore it for now.


More importantly, it will longjmp with the wrong source location.  Note that 
float_invalid_op_excp is attempting to use __always_inline__ to grab a correct 
value for GETPC.  Which, as one can predict, is doomed to failure.  As here we 
are, in another function which is not __always_inline__, producing the wrong value.


We need to drop all of the __always_inline__ nonsense, and properly pass down a 
value for GETPC from the top-level helper, so that we always think about it.



r~



[Qemu-devel] [PATCH 1/3] target/ppc: fmadd check for excp independently

2017-03-02 Thread Nikunj A Dadhania
Current order of checking does not confirm with the spec
(ISA 3.0: MultiplyAddDP page-469). Change the order and make them
independent of each other.

For example: a = infinity, b = zero, c = SNaN, this should set both
VXIMZ and VXNAN

Signed-off-by: Nikunj A Dadhania 
---
 target/ppc/fpu_helper.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 0535ad0..a547f58 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -747,17 +747,21 @@ static void float64_maddsub_update_excp(CPUPPCState *env, 
float64 arg1,
 float64 arg2, float64 arg3,
 unsigned int madd_flags)
 {
+if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
+ float64_is_signaling_nan(arg2, &env->fp_status) ||
+ float64_is_signaling_nan(arg3, &env->fp_status))) {
+/* sNaN operation */
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
+}
+
 if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
  (float64_is_zero(arg1) && float64_is_infinity(arg2 {
 /* Multiplication of zero by infinity */
-arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
-} else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
-float64_is_signaling_nan(arg2, &env->fp_status) ||
-float64_is_signaling_nan(arg3, &env->fp_status))) {
-/* sNaN operation */
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
-} else if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
-   float64_is_infinity(arg3)) {
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
+}
+
+if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
+float64_is_infinity(arg3)) {
 uint8_t aSign, bSign, cSign;
 
 aSign = float64_is_neg(arg1);
-- 
2.7.4