Skip to content

[RFC] Fix FPSCR.FI flag not being handled correctly #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vcoracolombo wants to merge 6 commits into from
Closed

[RFC] Fix FPSCR.FI flag not being handled correctly #76

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f40cb2
target/ppc: Fix FPSCR.FI flag for VSX_CVT_INT_TO_FP helper
vcoracolombo Mar 7, 2022
6dd0995
target/ppc: Fix FPSCR.FI flag for VSX_MADD helper
vcoracolombo Mar 7, 2022
b276447
fixup! target/ppc: Fix FPSCR.FI flag for VSX_CVT_INT_TO_FP helper
vcoracolombo Mar 23, 2022
8c6e2a6
fixup! target/ppc: Fix FPSCR.FI flag for VSX_MADD helper
vcoracolombo Mar 23, 2022
470df3f
fixup! target/ppc: Fix FPSCR.FI flag for VSX_CVT_INT_TO_FP helper
vcoracolombo Mar 24, 2022
70ca489
fixup! fixup! target/ppc: Fix FPSCR.FI flag for VSX_MADD helper
vcoracolombo Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 177 additions & 46 deletions target/ppc/fpu_helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,24 @@ static void finish_invalid_op_excp(CPUPPCState *env, int op, uintptr_t retaddr)
}
}

/* TODO: this is not a 'finish' anymore, find a better name */
static void finish_invalid_op_excp2(CPUPPCState *env, int op)
{
/* Update the floating-point invalid operation summary */
env->fpscr |= FP_VX;
/* Update the floating-point exception summary */
env->fpscr |= FP_FX;
if (env->fpscr & FP_VE) {
/* Update the floating-point enabled exception summary */
env->fpscr |= FP_FEX;

CPUState *cs = env_cpu(env);
cs->exception_index = POWERPC_EXCP_PROGRAM;
env->error_code = POWERPC_EXCP_FP | op;
/* Exception is deferred */
}
}

static void finish_invalid_op_arith(CPUPPCState *env, int op,
bool set_fpcc, uintptr_t retaddr)
{
Expand All @@ -225,13 +243,32 @@ static void finish_invalid_op_arith(CPUPPCState *env, int op,
finish_invalid_op_excp(env, op, retaddr);
}

/* TODO: this is not a 'finish' anymore, find a better name */
static void finish_invalid_op_arith2(CPUPPCState *env, int op, bool set_fpcc)
{
if (fpscr_ve == 0) {
Copy link

@alqotel alqotel Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this one when pointing the fpscr_* to change, this one should be env->fpscr & FP_VE too

if (set_fpcc) {
env->fpscr &= ~FP_FPCC;
env->fpscr |= (FP_C | FP_FU);
}
}
finish_invalid_op_excp2(env, op);
}

/* Signalling NaN */
static void float_invalid_op_vxsnan(CPUPPCState *env, uintptr_t retaddr)
{
env->fpscr |= FP_VXSNAN;
finish_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, retaddr);
}

/* TODO: Rename this to float_invalid_op_vxsnan when conversion is over */
static void float_invalid_op_vxsnan2(CPUPPCState *env)
{
env->fpscr |= FP_VXSNAN;
finish_invalid_op_excp2(env, POWERPC_EXCP_FP_VXSNAN);
}

/* Magnitude subtraction of infinities */
static void float_invalid_op_vxisi(CPUPPCState *env, bool set_fpcc,
uintptr_t retaddr)
Expand All @@ -240,6 +277,13 @@ static void float_invalid_op_vxisi(CPUPPCState *env, bool set_fpcc,
finish_invalid_op_arith(env, POWERPC_EXCP_FP_VXISI, set_fpcc, retaddr);
}

/* TODO: Rename this to float_invalid_op_vxisi when conversion is over */
static void float_invalid_op_vxisi2(CPUPPCState *env, bool set_fpcc)
{
env->fpscr |= FP_VXISI;
finish_invalid_op_arith2(env, POWERPC_EXCP_FP_VXISI, set_fpcc);
}

/* Division of infinity by infinity */
static void float_invalid_op_vxidi(CPUPPCState *env, bool set_fpcc,
uintptr_t retaddr)
Expand All @@ -264,6 +308,13 @@ static void float_invalid_op_vximz(CPUPPCState *env, bool set_fpcc,
finish_invalid_op_arith(env, POWERPC_EXCP_FP_VXIMZ, set_fpcc, retaddr);
}

/* TODO: Rename this to float_invalid_op_vximz when conversion is over */
static void float_invalid_op_vximz2(CPUPPCState *env, bool set_fpcc)
{
env->fpscr |= FP_VXIMZ;
finish_invalid_op_arith2(env, POWERPC_EXCP_FP_VXIMZ, set_fpcc);
}

/* Square root of a negative number */
static void float_invalid_op_vxsqrt(CPUPPCState *env, bool set_fpcc,
uintptr_t retaddr)
Expand Down Expand Up @@ -383,6 +434,22 @@ static inline void float_inexact_excp(CPUPPCState *env)
}
}

static inline void float_inexact_excp2(CPUPPCState *env)
Copy link
Author

@vcoracolombo vcoracolombo Mar 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is just a placeholder to avoid changing the original one and break other functions that were not addressed yet. The idea is to remove the old implementation when everything is converted to use update_fpscr

{
CPUState *cs = env_cpu(env);

env->fpscr |= FP_XX;
/* Update the floating-point exception summary */
env->fpscr |= FP_FX;
if (env->fpscr & FP_XE) {
/* Update the floating-point enabled exception summary */
env->fpscr |= FP_FEX;
/* We must update the target FPR before raising the exception */
cs->exception_index = POWERPC_EXCP_PROGRAM;
env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_XX;
}
}

void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
{
uint32_t mask = 1u << bit;
Expand Down Expand Up @@ -462,6 +529,51 @@ void helper_fpscr_check_status(CPUPPCState *env)
}
}

static void finish_fp_operation(CPUPPCState *env, int mask, uintptr_t raddr)
{
CPUState *cs = env_cpu(env);
int flags = get_float_exception_flags(&env->fp_status);

if (flags & float_flag_invalid) {
if ((mask & FP_VXIMZ) && (flags & float_flag_invalid_imz)) {
float_invalid_op_vximz2(env, mask & FP_FPRF);
}
if ((mask & FP_VXISI) && (flags & float_flag_invalid_isi)) {
float_invalid_op_vxisi2(env, mask & FP_FPRF);
}
if ((mask & FP_VXSNAN) && (flags & float_flag_invalid_snan)) {
float_invalid_op_vxsnan2(env);
}

/* TODO: Add other invalid exceptions */
}

if ((mask & FP_OX) && (flags & float_flag_overflow)) {
float_overflow_excp(env);
}
if ((mask & FP_UX) && (flags & float_flag_underflow)) {
float_underflow_excp(env);
}
if (mask & FP_FI) {
if (flags & float_flag_inexact) {
env->fpscr |= FP_FI;
} else {
env->fpscr &= ~FP_FI;
}
}
if ((mask & FP_XX) && (flags & float_flag_inexact)) {
float_inexact_excp2(env);
}

if (cs->exception_index == POWERPC_EXCP_PROGRAM &&
(env->error_code & POWERPC_EXCP_FP)) {
if (fp_exceptions_enabled(env)) {
raise_exception_err_ra(env, cs->exception_index,
env->error_code, raddr);
}
}
}

static void do_float_check_status(CPUPPCState *env, uintptr_t raddr)
{
CPUState *cs = env_cpu(env);
Expand Down Expand Up @@ -2155,9 +2267,8 @@ VSX_TSQRT(xvtsqrtsp, 4, float32, VsrW(i), -126, 23)
* fld - vsr_t field (VsrD(*) or VsrW(*))
* maddflgs - flags for the float*muladd routine that control the
* various forms (madd, msub, nmadd, nmsub)
* sfprf - set FPRF
*/
#define VSX_MADD(op, nels, tp, fld, maddflgs, sfprf) \
#define VSX_MADD(op, nels, tp, fld, maddflgs, fpscr_mask) \
void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \
ppc_vsr_t *s1, ppc_vsr_t *s2, ppc_vsr_t *s3) \
{ \
Expand All @@ -2172,37 +2283,49 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \
t.fld = tp##_muladd(s1->fld, s3->fld, s2->fld, maddflgs, &tstat); \
env->fp_status.float_exception_flags |= tstat.float_exception_flags; \
\
if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \
float_invalid_op_madd(env, tstat.float_exception_flags, \
sfprf, GETPC()); \
} \
\
if (sfprf) { \
if (fpscr_mask & FP_FPRF) { \
helper_compute_fprf_float64(env, t.fld); \
} \
} \
\
*xt = t; \
do_float_check_status(env, GETPC()); \
}

VSX_MADD(XSMADDDP, 1, float64, VsrD(0), MADD_FLGS, 1)
VSX_MADD(XSMSUBDP, 1, float64, VsrD(0), MSUB_FLGS, 1)
VSX_MADD(XSNMADDDP, 1, float64, VsrD(0), NMADD_FLGS, 1)
VSX_MADD(XSNMSUBDP, 1, float64, VsrD(0), NMSUB_FLGS, 1)
VSX_MADD(XSMADDSP, 1, float64r32, VsrD(0), MADD_FLGS, 1)
VSX_MADD(XSMSUBSP, 1, float64r32, VsrD(0), MSUB_FLGS, 1)
VSX_MADD(XSNMADDSP, 1, float64r32, VsrD(0), NMADD_FLGS, 1)
VSX_MADD(XSNMSUBSP, 1, float64r32, VsrD(0), NMSUB_FLGS, 1)

VSX_MADD(xvmadddp, 2, float64, VsrD(i), MADD_FLGS, 0)
VSX_MADD(xvmsubdp, 2, float64, VsrD(i), MSUB_FLGS, 0)
VSX_MADD(xvnmadddp, 2, float64, VsrD(i), NMADD_FLGS, 0)
VSX_MADD(xvnmsubdp, 2, float64, VsrD(i), NMSUB_FLGS, 0)

VSX_MADD(xvmaddsp, 4, float32, VsrW(i), MADD_FLGS, 0)
VSX_MADD(xvmsubsp, 4, float32, VsrW(i), MSUB_FLGS, 0)
VSX_MADD(xvnmaddsp, 4, float32, VsrW(i), NMADD_FLGS, 0)
VSX_MADD(xvnmsubsp, 4, float32, VsrW(i), NMSUB_FLGS, 0)
finish_fp_operation(env, fpscr_mask, GETPC()); \
}

VSX_MADD(XSMADDDP, 1, float64, VsrD(0), MADD_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSMSUBDP, 1, float64, VsrD(0), MSUB_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSNMADDDP, 1, float64, VsrD(0), NMADD_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSNMSUBDP, 1, float64, VsrD(0), NMSUB_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSMADDSP, 1, float64r32, VsrD(0), MADD_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSMSUBSP, 1, float64r32, VsrD(0), MSUB_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSNMADDSP, 1, float64r32, VsrD(0), NMADD_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))
VSX_MADD(XSNMSUBSP, 1, float64r32, VsrD(0), NMSUB_FLGS, \
(FP_FPRF | FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_FI | FP_XX | FP_UX | FP_OX))

VSX_MADD(xvmadddp, 2, float64, VsrD(i), MADD_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))
VSX_MADD(xvmsubdp, 2, float64, VsrD(i), MSUB_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))
VSX_MADD(xvnmadddp, 2, float64, VsrD(i), NMADD_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))
VSX_MADD(xvnmsubdp, 2, float64, VsrD(i), NMSUB_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))

VSX_MADD(xvmaddsp, 4, float32, VsrW(i), MADD_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))
VSX_MADD(xvmsubsp, 4, float32, VsrW(i), MSUB_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))
VSX_MADD(xvnmaddsp, 4, float32, VsrW(i), NMADD_FLGS, \
(FP_VXSNAN | FP_VXISI | FP_VXIMZ | FP_XX | FP_UX | FP_OX))
VSX_MADD(xvnmsubsp, 4, float32, VsrW(i), NMSUB_FLGS, \
(FP_VXSNAN | FP_XX | FP_UX | FP_OX))

/*
* VSX_MADDQ - VSX floating point quad-precision muliply/add
Expand Down Expand Up @@ -3006,10 +3129,8 @@ VSX_CVT_FP_TO_INT_VECTOR(xscvqpuwz, float128, uint32, f128, VsrD(0), 0x0ULL)
* ttp - target type (float32 or float64)
* sfld - source vsr_t field
* tfld - target vsr_t field
* jdef - definition of the j index (i or 2*i)
* sfprf - set FPRF
*/
Copy link
Author

@vcoracolombo vcoracolombo Mar 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This RFC is still missing comments that should be added for a final version (here, it's missing the fpscr_mask documentation)

#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, sfprf, r2sp) \
#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, r2sp, fpscr_mask) \
void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
{ \
ppc_vsr_t t = { }; \
Expand All @@ -3020,25 +3141,35 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
if (r2sp) { \
t.tfld = do_frsp(env, t.tfld, GETPC()); \
} \
if (sfprf) { \
if (fpscr_mask & FP_FPRF) { \
helper_compute_fprf_float64(env, t.tfld); \
} \
} \
\
*xt = t; \
do_float_check_status(env, GETPC()); \
}

VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, VsrD(0), VsrD(0), 1, 0)
VSX_CVT_INT_TO_FP(xscvuxddp, 1, uint64, float64, VsrD(0), VsrD(0), 1, 0)
VSX_CVT_INT_TO_FP(xscvsxdsp, 1, int64, float64, VsrD(0), VsrD(0), 1, 1)
VSX_CVT_INT_TO_FP(xscvuxdsp, 1, uint64, float64, VsrD(0), VsrD(0), 1, 1)
VSX_CVT_INT_TO_FP(xvcvsxddp, 2, int64, float64, VsrD(i), VsrD(i), 0, 0)
VSX_CVT_INT_TO_FP(xvcvuxddp, 2, uint64, float64, VsrD(i), VsrD(i), 0, 0)
VSX_CVT_INT_TO_FP(xvcvsxwdp, 2, int32, float64, VsrW(2 * i), VsrD(i), 0, 0)
VSX_CVT_INT_TO_FP(xvcvuxwdp, 2, uint64, float64, VsrW(2 * i), VsrD(i), 0, 0)
VSX_CVT_INT_TO_FP(xvcvsxwsp, 4, int32, float32, VsrW(i), VsrW(i), 0, 0)
VSX_CVT_INT_TO_FP(xvcvuxwsp, 4, uint32, float32, VsrW(i), VsrW(i), 0, 0)
finish_fp_operation(env, fpscr_mask, GETPC()); \
}

VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, VsrD(0), VsrD(0), 0, \
(FP_FPRF | FP_FI | FP_XX))
VSX_CVT_INT_TO_FP(xscvuxddp, 1, uint64, float64, VsrD(0), VsrD(0), 0, \
(FP_FPRF | FP_FI | FP_XX))
VSX_CVT_INT_TO_FP(xscvsxdsp, 1, int64, float64, VsrD(0), VsrD(0), 1, \
(FP_FPRF | FP_FI | FP_XX))
VSX_CVT_INT_TO_FP(xscvuxdsp, 1, uint64, float64, VsrD(0), VsrD(0), 1, \
(FP_FPRF | FP_FI | FP_XX))
VSX_CVT_INT_TO_FP(xvcvsxddp, 2, int64, float64, VsrD(i), VsrD(i), 0, \
FP_XX)
VSX_CVT_INT_TO_FP(xvcvuxddp, 2, uint64, float64, VsrD(i), VsrD(i), 0, \
FP_XX)
VSX_CVT_INT_TO_FP(xvcvsxwdp, 2, int32, float64, VsrW(2 * i), VsrD(i), 0, \
FP_XX)
VSX_CVT_INT_TO_FP(xvcvuxwdp, 2, uint64, float64, VsrW(2 * i), VsrD(i), 0, \
FP_XX)
VSX_CVT_INT_TO_FP(xvcvsxwsp, 4, int32, float32, VsrW(i), VsrW(i), 0, \
FP_XX)
VSX_CVT_INT_TO_FP(xvcvuxwsp, 4, uint32, float32, VsrW(i), VsrW(i), 0, \
FP_XX)

#define VSX_CVT_INT_TO_FP2(op, stp, ttp) \
void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
Expand Down