On 22/02/2022 21:20, Richard Henderson wrote:> On 2/22/22 04:36,
matheus.fe...@eldorado.org.br wrote:
From: Víctor Colombo <victor.colo...@eldorado.org.br>
Refactor VSX_SCALAR_CMP_DP, changing its name to VSX_SCALAR_CMP and
prepare the helper to be used for quadword comparisons.
Signed-off-by: Víctor Colombo <victor.colo...@eldorado.org.br>
Signed-off-by: Matheus Ferst <matheus.fe...@eldorado.org.br>
---
target/ppc/fpu_helper.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 9b034d1fe4..5ebbcfe3b7 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2265,28 +2265,30 @@ VSX_MADDQ(XSNMSUBQP, NMSUB_FLGS, 0)
VSX_MADDQ(XSNMSUBQPO, NMSUB_FLGS, 0)
/*
- * VSX_SCALAR_CMP_DP - VSX scalar floating point compare double
precision
+ * VSX_SCALAR_CMP - VSX scalar floating point compare
* op - instruction mnemonic
+ * tp - type
* cmp - comparison operation
* exp - expected result of comparison
+ * fld - vsr_t field
* svxvc - set VXVC bit
*/
-#define VSX_SCALAR_CMP_DP(op, cmp, exp,
svxvc) \
+#define VSX_SCALAR_CMP(op, tp, cmp, fld, exp,
svxvc) \
void helper_##op(CPUPPCState *env, ppc_vsr_t
*xt, \
ppc_vsr_t *xa, ppc_vsr_t
*xb) \
{
\
- ppc_vsr_t t =
*xt; \
+ ppc_vsr_t t = {
}; \
bool vxsnan_flag = false, vxvc_flag = false, vex_flag =
false; \
\
- if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status)
|| \
- float64_is_signaling_nan(xb->VsrD(0), &env->fp_status))
{ \
+ if (tp##_is_signaling_nan(xa->fld, &env->fp_status)
|| \
+ tp##_is_signaling_nan(xb->fld, &env->fp_status))
{ \
vxsnan_flag =
true; \
if (fpscr_ve == 0 && svxvc)
{ \
vxvc_flag =
true; \
} \
} else if (svxvc)
{ \
- vxvc_flag = float64_is_quiet_nan(xa->VsrD(0),
&env->fp_status) || \
- float64_is_quiet_nan(xb->VsrD(0),
&env->fp_status); \
+ vxvc_flag = tp##_is_quiet_nan(xa->fld, &env->fp_status)
|| \
+ tp##_is_quiet_nan(xb->fld,
&env->fp_status); \
}
Note that this can be simplified further, using the full FloatRelation
result and
float_flag_invalid_snan.
Note that do_scalar_cmp gets half-way there, only checking for NaNs once
we have
float_relation_unordered as a comparision result. But it could go
further and check
float_flag_invalid_snan and drop all of the other checks vs snan and qnan.
r~
Hello Richard! Thanks for your review
Could you please elaborate more on how do you think using
float*_compare and its FloatRelation result would work here?
I noticed do_scalar_cmp modifies CR and sets FPCC flag, which
is not what VSX_SCALAR_CMP do. Using that function would require a
rework.
An option I though would be to bring into VSX_SCALAR_CMP the
important necessary parts, something like this:
#define VSX_SCALAR_CMP(op, tp, cmp, fld, svxvc, expr)
...
r = tp##_compare(xa->fld, xb->fld, &env->fp_status);
\
if (expr) {
\
memset(&t.fld, 0xFF, sizeof(t.fld));
\
} else if (r == float_relation_unordered) {
\
if (env->fp_status.float_exception_flags &
float_flag_invalid_snan) { \
float_invalid_op_vxsnan(env, GETPC());
\
if (fpscr_ve == 0 && svxvc) {
\
float_invalid_op_vxvc(env, 0, GETPC());
\
}
\
} else if (svxvc) {
\
if (tp##_is_quiet_nan(xa->fld, &env->fp_status) ||
\
tp##_is_quiet_nan(xb->fld, &env->fp_status)) {
\
float_invalid_op_vxvc(env, 0, GETPC());
\
}
\
}
\
}
\
...
VSX_SCALAR_CMP(XSCMPEQDP, float64, eq, VsrD(0), 0, r ==
float_relation_equal)
VSX_SCALAR_CMP(XSCMPGEDP, float64, le, VsrD(0), 1, \
r == float_relation_equal || r == float_relation_greater)
VSX_SCALAR_CMP(XSCMPGTDP, float64, lt, VsrD(0), 1, r ==
float_relation_greater)
But this still looks convoluted. Another option I came with would be:
ppc_vsr_t t = { };
\
\
helper_reset_fpstatus(env);
\
\
if (tp##_##cmp##_quiet(xb->fld, xa->fld, &env->fp_status)) {
\
memset(&t.fld, 0xFF, sizeof(t.fld));
\
}
\
\
if (env->fp_status.float_exception_flags & float_flag_invalid_snan)
{ \
float_invalid_op_vxsnan(env, GETPC());
\
if (fpscr_ve == 0 && svxvc) {
\
float_invalid_op_vxvc(env, 0, GETPC());
\
}
\
} else if (svxvc) {
\
if (tp##_is_quiet_nan(xa->fld, &env->fp_status) ||
\
tp##_is_quiet_nan(xb->fld, &env->fp_status)) {
\
float_invalid_op_vxvc(env, 0, GETPC());
\
}
\
}
\
Is this close to what you were thinking?
Thank you very much!
-- Víctor
