| Message ID | 1475866623-16841-5-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 10/07/2016 01:57 PM, Nikunj A Dadhania wrote: > +#define VSX_SCALAR_CMP_DP(op, cmp, exp, svxvc) \ > +void helper_##op(CPUPPCState *env, uint32_t opcode) \ > +{ \ > + ppc_vsr_t xt, xa, xb; \ > + \ > + getVSR(xA(opcode), &xa, env); \ > + getVSR(xB(opcode), &xb, env); \ > + getVSR(xT(opcode), &xt, env); \ > + \ > + if (unlikely(float64_is_any_nan(xa.VsrD(0)) || \ > + float64_is_any_nan(xb.VsrD(0)))) { \ > + if (float64_is_signaling_nan(xa.VsrD(0), &env->fp_status) || \ > + float64_is_signaling_nan(xb.VsrD(0), &env->fp_status)) { \ > + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0); \ > + } \ > + if (svxvc) { \ > + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXVC, 0); \ > + } \ > + } else { \ > + if (float64_##cmp(xb.VsrD(0), xa.VsrD(0), &env->fp_status) == exp) { \ > + if (msr_le) { \ > + xt.VsrD(0) = 0; \ > + xt.VsrD(1) = -1; \ > + } else { \ > + xt.VsrD(0) = -1; \ > + xt.VsrD(1) = 0; \ > + } \ > + } else { \ > + xt.VsrD(0) = 0; \ > + xt.VsrD(1) = 0; \ > + } \ > + } \ > + \ > + putVSR(xT(opcode), &xt, env); \ > + helper_float_check_status(env); \ > +} I think you should be checking for NaN after the compare, and seeing that env->fp_status.float_exception_flags is non-zero. C.f. FPU_FCTI. Or in general, the coding structure used by target-tricore: result = float*op(args...) flags = get_float_exception_flags(&env->fp_status); if (unlikely(flags)) { set_float_exception_flags(&env->fp_status, 0); // special cases for nans, sometimes modifying result float_check_status(env, flags, GETPC()) } return result // or putVSR as appropriate Of course, the same can be said for other places in fpu_helper.c, where this detail has been previously missed. And, unrelated, but a reminder for future cleanup: the fmadd, fmsub, fnmadd, and fnmsub helpers should be rewritten to use float64_muladd. To be fair, I think these were written before we had proper fused multiply-add support within softfloat. r~
Richard Henderson <rth@twiddle.net> writes: > On 10/07/2016 01:57 PM, Nikunj A Dadhania wrote: >> +#define VSX_SCALAR_CMP_DP(op, cmp, exp, svxvc) \ >> +void helper_##op(CPUPPCState *env, uint32_t opcode) \ >> +{ \ >> + ppc_vsr_t xt, xa, xb; \ >> + \ >> + getVSR(xA(opcode), &xa, env); \ >> + getVSR(xB(opcode), &xb, env); \ >> + getVSR(xT(opcode), &xt, env); \ >> + \ >> + if (unlikely(float64_is_any_nan(xa.VsrD(0)) || \ >> + float64_is_any_nan(xb.VsrD(0)))) { \ >> + if (float64_is_signaling_nan(xa.VsrD(0), &env->fp_status) || \ >> + float64_is_signaling_nan(xb.VsrD(0), &env->fp_status)) { \ >> + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0); \ >> + } \ >> + if (svxvc) { \ >> + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXVC, 0); \ >> + } \ >> + } else { \ >> + if (float64_##cmp(xb.VsrD(0), xa.VsrD(0), &env->fp_status) == exp) { \ >> + if (msr_le) { \ >> + xt.VsrD(0) = 0; \ >> + xt.VsrD(1) = -1; \ >> + } else { \ >> + xt.VsrD(0) = -1; \ >> + xt.VsrD(1) = 0; \ >> + } \ >> + } else { \ >> + xt.VsrD(0) = 0; \ >> + xt.VsrD(1) = 0; \ >> + } \ >> + } \ >> + \ >> + putVSR(xT(opcode), &xt, env); \ >> + helper_float_check_status(env); \ >> +} > > I think you should be checking for NaN after the compare, and seeing that > env->fp_status.float_exception_flags is non-zero. C.f. FPU_FCTI. Or in > general, the coding structure used by target-tricore: > > result = float*op(args...) > flags = get_float_exception_flags(&env->fp_status); > if (unlikely(flags)) { > set_float_exception_flags(&env->fp_status, 0); > // special cases for nans, sometimes modifying result > float_check_status(env, flags, GETPC()) > } > return result // or putVSR as appropriate > > Of course, the same can be said for other places in fpu_helper.c, where this > detail has been previously missed. Yes, I had noticed that, but didn't want to change the behaviour as I am not expert here. I will update and send a new revision. > And, unrelated, but a reminder for future cleanup: the fmadd, fmsub, fnmadd, > and fnmsub helpers should be rewritten to use float64_muladd. To be fair, I > think these were written before we had proper fused multiply-add support within > softfloat. Sure. Will add that to my todo list. Regards Nikunj
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c index b0760f0..87c17d9 100644 --- a/target-ppc/fpu_helper.c +++ b/target-ppc/fpu_helper.c @@ -2362,6 +2362,54 @@ VSX_MADD(xvnmaddmsp, 4, float32, VsrW(i), NMADD_FLGS, 0, 0, 0) VSX_MADD(xvnmsubasp, 4, float32, VsrW(i), NMSUB_FLGS, 1, 0, 0) VSX_MADD(xvnmsubmsp, 4, float32, VsrW(i), NMSUB_FLGS, 0, 0, 0) +/* VSX_SCALAR_CMP_DP - VSX scalar floating point compare double precision + * op - instruction mnemonic + * cmp - comparison operation + * exp - expected result of comparison + * svxvc - set VXVC bit + */ +#define VSX_SCALAR_CMP_DP(op, cmp, exp, svxvc) \ +void helper_##op(CPUPPCState *env, uint32_t opcode) \ +{ \ + ppc_vsr_t xt, xa, xb; \ + \ + getVSR(xA(opcode), &xa, env); \ + getVSR(xB(opcode), &xb, env); \ + getVSR(xT(opcode), &xt, env); \ + \ + if (unlikely(float64_is_any_nan(xa.VsrD(0)) || \ + float64_is_any_nan(xb.VsrD(0)))) { \ + if (float64_is_signaling_nan(xa.VsrD(0), &env->fp_status) || \ + float64_is_signaling_nan(xb.VsrD(0), &env->fp_status)) { \ + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0); \ + } \ + if (svxvc) { \ + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXVC, 0); \ + } \ + } else { \ + if (float64_##cmp(xb.VsrD(0), xa.VsrD(0), &env->fp_status) == exp) { \ + if (msr_le) { \ + xt.VsrD(0) = 0; \ + xt.VsrD(1) = -1; \ + } else { \ + xt.VsrD(0) = -1; \ + xt.VsrD(1) = 0; \ + } \ + } else { \ + xt.VsrD(0) = 0; \ + xt.VsrD(1) = 0; \ + } \ + } \ + \ + putVSR(xT(opcode), &xt, env); \ + helper_float_check_status(env); \ +} + +VSX_SCALAR_CMP_DP(xscmpeqdp, eq, 1, 0) +VSX_SCALAR_CMP_DP(xscmpgedp, le, 1, 1) +VSX_SCALAR_CMP_DP(xscmpgtdp, lt, 1, 1) +VSX_SCALAR_CMP_DP(xscmpnedp, eq, 0, 0) + #define VSX_SCALAR_CMP(op, ordered) \ void helper_##op(CPUPPCState *env, uint32_t opcode) \ { \ diff --git a/target-ppc/helper.h b/target-ppc/helper.h index 5fcc546..0337292 100644 --- a/target-ppc/helper.h +++ b/target-ppc/helper.h @@ -389,6 +389,10 @@ DEF_HELPER_2(xsnmaddadp, void, env, i32) DEF_HELPER_2(xsnmaddmdp, void, env, i32) DEF_HELPER_2(xsnmsubadp, void, env, i32) DEF_HELPER_2(xsnmsubmdp, void, env, i32) +DEF_HELPER_2(xscmpeqdp, void, env, i32) +DEF_HELPER_2(xscmpgtdp, void, env, i32) +DEF_HELPER_2(xscmpgedp, void, env, i32) +DEF_HELPER_2(xscmpnedp, void, env, i32) DEF_HELPER_2(xscmpodp, void, env, i32) DEF_HELPER_2(xscmpudp, void, env, i32) DEF_HELPER_2(xsmaxdp, void, env, i32) diff --git a/target-ppc/translate/vsx-impl.inc.c b/target-ppc/translate/vsx-impl.inc.c index d510842..3ee20b4 100644 --- a/target-ppc/translate/vsx-impl.inc.c +++ b/target-ppc/translate/vsx-impl.inc.c @@ -620,6 +620,10 @@ GEN_VSX_HELPER_2(xsnmaddadp, 0x04, 0x14, 0, PPC2_VSX) GEN_VSX_HELPER_2(xsnmaddmdp, 0x04, 0x15, 0, PPC2_VSX) GEN_VSX_HELPER_2(xsnmsubadp, 0x04, 0x16, 0, PPC2_VSX) GEN_VSX_HELPER_2(xsnmsubmdp, 0x04, 0x17, 0, PPC2_VSX) +GEN_VSX_HELPER_2(xscmpeqdp, 0x0C, 0x00, 0, PPC2_ISA300) +GEN_VSX_HELPER_2(xscmpgtdp, 0x0C, 0x01, 0, PPC2_ISA300) +GEN_VSX_HELPER_2(xscmpgedp, 0x0C, 0x02, 0, PPC2_ISA300) +GEN_VSX_HELPER_2(xscmpnedp, 0x0C, 0x03, 0, PPC2_ISA300) GEN_VSX_HELPER_2(xscmpodp, 0x0C, 0x05, 0, PPC2_VSX) GEN_VSX_HELPER_2(xscmpudp, 0x0C, 0x04, 0, PPC2_VSX) GEN_VSX_HELPER_2(xsmaxdp, 0x00, 0x14, 0, PPC2_VSX) diff --git a/target-ppc/translate/vsx-ops.inc.c b/target-ppc/translate/vsx-ops.inc.c index af0d27e..202c557 100644 --- a/target-ppc/translate/vsx-ops.inc.c +++ b/target-ppc/translate/vsx-ops.inc.c @@ -114,6 +114,10 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX), GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX), GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX), GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX), +GEN_XX3FORM(xscmpeqdp, 0x0C, 0x00, PPC2_ISA300), +GEN_XX3FORM(xscmpgtdp, 0x0C, 0x01, PPC2_ISA300), +GEN_XX3FORM(xscmpgedp, 0x0C, 0x02, PPC2_ISA300), +GEN_XX3FORM(xscmpnedp, 0x0C, 0x03, PPC2_ISA300), GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), GEN_XX2IFORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), GEN_XX3FORM(xsmaxdp, 0x00, 0x14, PPC2_VSX),