* [PATCH v2 0/8] Multiple ppc instructions fixes
@ 2022-09-06 12:55 Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 1/8] target/ppc: Remove extra space from s128 field in ppc_vsr_t Víctor Colombo
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
This patch set fixes multiple instructions for PPC targets that were
producing incorrect results, or setting the wrong bits in FPSCR.
Patch 1 is just a style fix, trivial.
Patch 8 adds helper_reset_fpstatus() calls to instructions
that have an issue where the exception flags are being kept from
the previous instruction, causing incorrect bits to be set,
specially the non-sticky FI bit.
Other patches fixes other specific situations.
v1->v2:
- Squash patches 8 through 19 and write a better commit message to it.
- Dropped Daniel's R-b in the squashed patches, as the squash merged
both reviewed and non-reviewed patches. Now require a new, single
R-b.
Víctor Colombo (8):
target/ppc: Remove extra space from s128 field in ppc_vsr_t
target/ppc: Remove unused xer_* macros
target/ppc: Zero second doubleword in DFP instructions
target/ppc: Set result to QNaN for DENBCD when VXCVI occurs
target/ppc: Zero second doubleword for VSX madd instructions
target/ppc: Set OV32 when OV is set
target/ppc: Zero second doubleword of VSR registers for FPR insns
target/ppc: Clear fpstatus flags on helpers missing it
target/ppc/cpu.h | 6 +-----
target/ppc/dfp_helper.c | 31 ++++++++++++++++++++++++++++---
target/ppc/fpu_helper.c | 39 +++++++++++++++++++++++++++------------
target/ppc/int_helper.c | 4 ++--
target/ppc/translate.c | 8 ++++++++
5 files changed, 66 insertions(+), 22 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/8] target/ppc: Remove extra space from s128 field in ppc_vsr_t
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 2/8] target/ppc: Remove unused xer_* macros Víctor Colombo
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
Very trivial rogue space removal. There are two spaces between Int128
and s128 in ppc_vsr_t struct, where it should be only one.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/cpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index a4c893cfad..985ff86f55 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -246,7 +246,7 @@ typedef union _ppc_vsr_t {
#ifdef CONFIG_INT128
__uint128_t u128;
#endif
- Int128 s128;
+ Int128 s128;
} ppc_vsr_t;
typedef ppc_vsr_t ppc_avr_t;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/8] target/ppc: Remove unused xer_* macros
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 1/8] target/ppc: Remove extra space from s128 field in ppc_vsr_t Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 3/8] target/ppc: Zero second doubleword in DFP instructions Víctor Colombo
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
The macros xer_ov, xer_ca, xer_ov32, and xer_ca32 are both unused and
hiding the usage of env. Remove them.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/cpu.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 985ff86f55..6481f48087 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1506,10 +1506,6 @@ void ppc_compat_add_property(Object *obj, const char *name,
#define XER_CMP 8
#define XER_BC 0
#define xer_so (env->so)
-#define xer_ov (env->ov)
-#define xer_ca (env->ca)
-#define xer_ov32 (env->ov)
-#define xer_ca32 (env->ca)
#define xer_cmp ((env->xer >> XER_CMP) & 0xFF)
#define xer_bc ((env->xer >> XER_BC) & 0x7F)
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/8] target/ppc: Zero second doubleword in DFP instructions
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 1/8] target/ppc: Remove extra space from s128 field in ppc_vsr_t Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 2/8] target/ppc: Remove unused xer_* macros Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 4/8] target/ppc: Set result to QNaN for DENBCD when VXCVI occurs Víctor Colombo
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
Starting at PowerISA v3.1, the second doubleword of the registers
used to store results in DFP instructions are supposed to be zeroed.
From the ISA, chapter 7.2.1.1 Floating-Point Registers:
"""
Chapter 4. Floating-Point Facility provides 32 64-bit
FPRs. Chapter 5. Decimal Floating-Point also employs
FPRs in decimal floating-point (DFP) operations. When
VSX is implemented, the 32 FPRs are mapped to
doubleword 0 of VSRs 0-31. (...)
All instructions that operate on an FPR are redefined
to operate on doubleword element 0 of the
corresponding VSR. (...)
and the contents of doubleword element 1 of the
VSR corresponding to the target FPR or FPR pair for these
instructions are set to 0.
"""
Before, the result stored at doubleword 1 was said to be undefined.
With that, this patch changes the DFP facility to zero doubleword 1
when using set_dfp64 and set_dfp128. This fixes the behavior for ISA
3.1 while keeping the behavior correct for previous ones.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/dfp_helper.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index 5ba74b2124..be7aa5357a 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -42,13 +42,16 @@ static void get_dfp128(ppc_vsr_t *dst, ppc_fprp_t *dfp)
static void set_dfp64(ppc_fprp_t *dfp, ppc_vsr_t *src)
{
- dfp->VsrD(0) = src->VsrD(1);
+ dfp[0].VsrD(0) = src->VsrD(1);
+ dfp[0].VsrD(1) = 0ULL;
}
static void set_dfp128(ppc_fprp_t *dfp, ppc_vsr_t *src)
{
dfp[0].VsrD(0) = src->VsrD(0);
dfp[1].VsrD(0) = src->VsrD(1);
+ dfp[0].VsrD(1) = 0ULL;
+ dfp[1].VsrD(1) = 0ULL;
}
static void set_dfp128_to_avr(ppc_avr_t *dst, ppc_vsr_t *src)
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/8] target/ppc: Set result to QNaN for DENBCD when VXCVI occurs
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
` (2 preceding siblings ...)
2022-09-06 12:55 ` [PATCH v2 3/8] target/ppc: Zero second doubleword in DFP instructions Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 5/8] target/ppc: Zero second doubleword for VSX madd instructions Víctor Colombo
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
According to the ISA, for instruction DENBCD:
"If an invalid BCD digit or sign code is detected in the source
operand, an invalid-operation exception (VXCVI) occurs."
In the Invalid Operation Exception section, there is the situation:
"When Invalid Operation Exception is disabled (VE=0) and Invalid
Operation occurs (...) If the operation is an (...) or format the
target FPR is set to a Quiet NaN". This was not being done in
QEMU.
This patch sets the result to QNaN when the instruction DENBCD causes
an Invalid Operation Exception.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/dfp_helper.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index be7aa5357a..cc024316d5 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -1147,6 +1147,26 @@ static inline uint8_t dfp_get_bcd_digit_128(ppc_vsr_t *t, unsigned n)
return t->VsrD((n & 0x10) ? 0 : 1) >> ((n << 2) & 63) & 15;
}
+static inline void dfp_invalid_op_vxcvi_64(struct PPC_DFP *dfp)
+{
+ /* TODO: fpscr is incorrectly not being saved to env */
+ dfp_set_FPSCR_flag(dfp, FP_VX | FP_VXCVI, FPSCR_VE);
+ if ((dfp->env->fpscr & FP_VE) == 0) {
+ dfp->vt.VsrD(1) = 0x7c00000000000000; /* QNaN */
+ }
+}
+
+
+static inline void dfp_invalid_op_vxcvi_128(struct PPC_DFP *dfp)
+{
+ /* TODO: fpscr is incorrectly not being saved to env */
+ dfp_set_FPSCR_flag(dfp, FP_VX | FP_VXCVI, FPSCR_VE);
+ if ((dfp->env->fpscr & FP_VE) == 0) {
+ dfp->vt.VsrD(0) = 0x7c00000000000000; /* QNaN */
+ dfp->vt.VsrD(1) = 0x0;
+ }
+}
+
#define DFP_HELPER_ENBCD(op, size) \
void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
uint32_t s) \
@@ -1173,7 +1193,8 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
sgn = 0; \
break; \
default: \
- dfp_set_FPSCR_flag(&dfp, FP_VX | FP_VXCVI, FPSCR_VE); \
+ dfp_invalid_op_vxcvi_##size(&dfp); \
+ set_dfp##size(t, &dfp.vt); \
return; \
} \
} \
@@ -1183,7 +1204,8 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
digits[(size) / 4 - n] = dfp_get_bcd_digit_##size(&dfp.vb, \
offset++); \
if (digits[(size) / 4 - n] > 10) { \
- dfp_set_FPSCR_flag(&dfp, FP_VX | FP_VXCVI, FPSCR_VE); \
+ dfp_invalid_op_vxcvi_##size(&dfp); \
+ set_dfp##size(t, &dfp.vt); \
return; \
} else { \
nonzero |= (digits[(size) / 4 - n] > 0); \
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/8] target/ppc: Zero second doubleword for VSX madd instructions
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
` (3 preceding siblings ...)
2022-09-06 12:55 ` [PATCH v2 4/8] target/ppc: Set result to QNaN for DENBCD when VXCVI occurs Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 6/8] target/ppc: Set OV32 when OV is set Víctor Colombo
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
In 205eb5a89e we updated most VSX instructions to zero the
second doubleword, as is requested by PowerISA since v3.1.
However, VSX_MADD helper was left behind unchanged, while it
is also affected and should be fixed as well.
This patch applies the fix for MADD instructions.
Fixes: 205eb5a89e ("target/ppc: Change VSX instructions behavior to fill with zeros")
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/fpu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 0f045b70f8..95b22d99b3 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2176,7 +2176,7 @@ VSX_TSQRT(xvtsqrtsp, 4, float32, VsrW(i), -126, 23)
void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \
ppc_vsr_t *s1, ppc_vsr_t *s2, ppc_vsr_t *s3) \
{ \
- ppc_vsr_t t = *xt; \
+ ppc_vsr_t t = { }; \
int i; \
\
helper_reset_fpstatus(env); \
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 6/8] target/ppc: Set OV32 when OV is set
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
` (4 preceding siblings ...)
2022-09-06 12:55 ` [PATCH v2 5/8] target/ppc: Zero second doubleword for VSX madd instructions Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 7/8] target/ppc: Zero second doubleword of VSR registers for FPR insns Víctor Colombo
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
According to PowerISA: "OV32 is set whenever OV is implicitly set, and
is set to the same value that OV is defined to be set to in 32-bit
mode".
This patch changes helper_update_ov_legacy to set/clear ov32 when
applicable.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/int_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index d905f07d02..696096100b 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -37,9 +37,9 @@
static inline void helper_update_ov_legacy(CPUPPCState *env, int ov)
{
if (unlikely(ov)) {
- env->so = env->ov = 1;
+ env->so = env->ov = env->ov32 = 1;
} else {
- env->ov = 0;
+ env->ov = env->ov32 = 0;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 7/8] target/ppc: Zero second doubleword of VSR registers for FPR insns
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
` (5 preceding siblings ...)
2022-09-06 12:55 ` [PATCH v2 6/8] target/ppc: Set OV32 when OV is set Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it Víctor Colombo
2022-09-06 20:07 ` [PATCH v2 0/8] Multiple ppc instructions fixes Daniel Henrique Barboza
8 siblings, 0 replies; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
FPR register are mapped to the first doubleword of the VSR registers.
Since PowerISA v3.1, the second doubleword of the target register
must be zeroed for FP instructions.
This patch does it by writting 0 to the second dw everytime the
first dw is being written using set_fpr.
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/translate.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 000b1e518d..5e433315e1 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6443,6 +6443,14 @@ static inline void get_fpr(TCGv_i64 dst, int regno)
static inline void set_fpr(int regno, TCGv_i64 src)
{
tcg_gen_st_i64(src, cpu_env, fpr_offset(regno));
+ /*
+ * Before PowerISA v3.1 the result of doubleword 1 of the VSR
+ * corresponding to the target FPR was undefined. However,
+ * most (if not all) real hardware were setting the result to 0.
+ * Starting at ISA v3.1, the result for doubleword 1 is now defined
+ * to be 0.
+ */
+ tcg_gen_st_i64(tcg_constant_i64(0), cpu_env, vsr64_offset(regno, false));
}
static inline void get_avr64(TCGv_i64 dst, int regno, bool high)
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
` (6 preceding siblings ...)
2022-09-06 12:55 ` [PATCH v2 7/8] target/ppc: Zero second doubleword of VSR registers for FPR insns Víctor Colombo
@ 2022-09-06 12:55 ` Víctor Colombo
2022-09-06 17:16 ` Daniel Henrique Barboza
2022-09-06 20:07 ` [PATCH v2 0/8] Multiple ppc instructions fixes Daniel Henrique Barboza
8 siblings, 1 reply; 11+ messages in thread
From: Víctor Colombo @ 2022-09-06 12:55 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, richard.henderson, victor.colombo,
matheus.ferst, lucas.araujo, leandro.lupori, lucas.coutinho
In ppc emulation, exception flags are not cleared at the end of an
instruction. Instead, the next instruction is responsible to clear
it before its emulation. However, some helpers are not doing it,
causing an issue where the previously set exception flags are being
used and leading to incorrect values being set in FPSCR.
Fix this by clearing fp_status before doing the instruction 'real' work
for the following helpers that were missing this behavior:
- VSX_CVT_INT_TO_FP_VECTOR
- VSX_CVT_FP_TO_FP
- VSX_CVT_FP_TO_INT_VECTOR
- VSX_CVT_FP_TO_INT2
- VSX_CVT_FP_TO_INT
- VSX_CVT_FP_TO_FP_HP
- VSX_CVT_FP_TO_FP_VECTOR
- VSX_CMP
- VSX_ROUND
- xscvqpdp
- xscvdpsp[n]
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
target/ppc/fpu_helper.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 95b22d99b3..331361234f 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2637,6 +2637,8 @@ uint32_t helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \
int all_true = 1; \
int all_false = 1; \
\
+ helper_reset_fpstatus(env); \
+ \
for (i = 0; i < nels; i++) { \
if (unlikely(tp##_is_any_nan(xa->fld) || \
tp##_is_any_nan(xb->fld))) { \
@@ -2690,6 +2692,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
ppc_vsr_t t = { }; \
int i; \
\
+ helper_reset_fpstatus(env); \
+ \
for (i = 0; i < nels; i++) { \
t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
if (unlikely(stp##_is_signaling_nan(xb->sfld, \
@@ -2715,6 +2719,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
ppc_vsr_t t = { }; \
int i; \
\
+ helper_reset_fpstatus(env); \
+ \
for (i = 0; i < nels; i++) { \
t.VsrW(2 * i) = stp##_to_##ttp(xb->VsrD(i), &env->fp_status); \
if (unlikely(stp##_is_signaling_nan(xb->VsrD(i), \
@@ -2752,6 +2758,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
ppc_vsr_t t = *xt; \
int i; \
\
+ helper_reset_fpstatus(env); \
+ \
for (i = 0; i < nels; i++) { \
t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
if (unlikely(stp##_is_signaling_nan(xb->sfld, \
@@ -2787,6 +2795,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
ppc_vsr_t t = { }; \
int i; \
\
+ helper_reset_fpstatus(env); \
+ \
for (i = 0; i < nels; i++) { \
t.tfld = stp##_to_##ttp(xb->sfld, 1, &env->fp_status); \
if (unlikely(stp##_is_signaling_nan(xb->sfld, \
@@ -2834,6 +2844,8 @@ void helper_XSCVQPDP(CPUPPCState *env, uint32_t ro, ppc_vsr_t *xt,
ppc_vsr_t t = { };
float_status tstat;
+ helper_reset_fpstatus(env);
+
tstat = env->fp_status;
if (ro != 0) {
tstat.float_rounding_mode = float_round_to_odd;
@@ -2855,6 +2867,7 @@ uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb)
{
uint64_t result, sign, exp, frac;
+ helper_reset_fpstatus(env);
float_status tstat = env->fp_status;
set_float_exception_flags(0, &tstat);
@@ -2910,22 +2923,20 @@ uint64_t helper_XSCVSPDPN(uint64_t xb)
#define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, sfi, rnan) \
void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
{ \
- int all_flags = env->fp_status.float_exception_flags, flags; \
ppc_vsr_t t = { }; \
- int i; \
+ int i, flags; \
+ \
+ helper_reset_fpstatus(env); \
\
for (i = 0; i < nels; i++) { \
- env->fp_status.float_exception_flags = 0; \
t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status); \
flags = env->fp_status.float_exception_flags; \
if (unlikely(flags & float_flag_invalid)) { \
t.tfld = float_invalid_cvt(env, flags, t.tfld, rnan, 0, GETPC());\
} \
- all_flags |= flags; \
} \
\
*xt = t; \
- env->fp_status.float_exception_flags = all_flags; \
do_float_check_status(env, sfi, GETPC()); \
}
@@ -2977,12 +2988,12 @@ VSX_CVT_FP_TO_INT128(XSCVQPSQZ, int128, 0x8000000000000000ULL);
#define VSX_CVT_FP_TO_INT2(op, nels, stp, ttp, sfi, rnan) \
void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
{ \
- int all_flags = env->fp_status.float_exception_flags, flags; \
ppc_vsr_t t = { }; \
- int i; \
+ int i, flags; \
+ \
+ helper_reset_fpstatus(env); \
\
for (i = 0; i < nels; i++) { \
- env->fp_status.float_exception_flags = 0; \
t.VsrW(2 * i) = stp##_to_##ttp##_round_to_zero(xb->VsrD(i), \
&env->fp_status); \
flags = env->fp_status.float_exception_flags; \
@@ -2991,11 +3002,9 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
rnan, 0, GETPC()); \
} \
t.VsrW(2 * i + 1) = t.VsrW(2 * i); \
- all_flags |= flags; \
} \
\
*xt = t; \
- env->fp_status.float_exception_flags = all_flags; \
do_float_check_status(env, sfi, GETPC()); \
}
@@ -3020,6 +3029,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
ppc_vsr_t t = { }; \
int flags; \
\
+ helper_reset_fpstatus(env); \
+ \
t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status); \
flags = get_float_exception_flags(&env->fp_status); \
if (flags & float_flag_invalid) { \
@@ -3032,7 +3043,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
VSX_CVT_FP_TO_INT_VECTOR(xscvqpsdz, float128, int64, f128, VsrD(0), \
0x8000000000000000ULL)
-
VSX_CVT_FP_TO_INT_VECTOR(xscvqpswz, float128, int32, f128, VsrD(0), \
0xffffffff80000000ULL)
VSX_CVT_FP_TO_INT_VECTOR(xscvqpudz, float128, uint64, f128, VsrD(0), 0x0ULL)
@@ -3055,6 +3065,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
ppc_vsr_t t = { }; \
int i; \
\
+ helper_reset_fpstatus(env); \
+ \
for (i = 0; i < nels; i++) { \
t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
if (r2sp) { \
@@ -3124,6 +3136,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
{ \
ppc_vsr_t t = *xt; \
\
+ helper_reset_fpstatus(env); \
t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
helper_compute_fprf_##ttp(env, t.tfld); \
\
@@ -3157,6 +3170,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
int i; \
FloatRoundMode curr_rounding_mode; \
\
+ helper_reset_fpstatus(env); \
+ \
if (rmode != FLOAT_ROUND_CURRENT) { \
curr_rounding_mode = get_float_rounding_mode(&env->fp_status); \
set_float_rounding_mode(rmode, &env->fp_status); \
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it
2022-09-06 12:55 ` [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it Víctor Colombo
@ 2022-09-06 17:16 ` Daniel Henrique Barboza
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2022-09-06 17:16 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, david, groug, richard.henderson, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
On 9/6/22 09:55, Víctor Colombo wrote:
> In ppc emulation, exception flags are not cleared at the end of an
> instruction. Instead, the next instruction is responsible to clear
> it before its emulation. However, some helpers are not doing it,
> causing an issue where the previously set exception flags are being
> used and leading to incorrect values being set in FPSCR.
> Fix this by clearing fp_status before doing the instruction 'real' work
> for the following helpers that were missing this behavior:
>
> - VSX_CVT_INT_TO_FP_VECTOR
> - VSX_CVT_FP_TO_FP
> - VSX_CVT_FP_TO_INT_VECTOR
> - VSX_CVT_FP_TO_INT2
> - VSX_CVT_FP_TO_INT
> - VSX_CVT_FP_TO_FP_HP
> - VSX_CVT_FP_TO_FP_VECTOR
> - VSX_CMP
> - VSX_ROUND
> - xscvqpdp
> - xscvdpsp[n]
>
> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> target/ppc/fpu_helper.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 95b22d99b3..331361234f 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -2637,6 +2637,8 @@ uint32_t helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \
> int all_true = 1; \
> int all_false = 1; \
> \
> + helper_reset_fpstatus(env); \
> + \
> for (i = 0; i < nels; i++) { \
> if (unlikely(tp##_is_any_nan(xa->fld) || \
> tp##_is_any_nan(xb->fld))) { \
> @@ -2690,6 +2692,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> ppc_vsr_t t = { }; \
> int i; \
> \
> + helper_reset_fpstatus(env); \
> + \
> for (i = 0; i < nels; i++) { \
> t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
> if (unlikely(stp##_is_signaling_nan(xb->sfld, \
> @@ -2715,6 +2719,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> ppc_vsr_t t = { }; \
> int i; \
> \
> + helper_reset_fpstatus(env); \
> + \
> for (i = 0; i < nels; i++) { \
> t.VsrW(2 * i) = stp##_to_##ttp(xb->VsrD(i), &env->fp_status); \
> if (unlikely(stp##_is_signaling_nan(xb->VsrD(i), \
> @@ -2752,6 +2758,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
> ppc_vsr_t t = *xt; \
> int i; \
> \
> + helper_reset_fpstatus(env); \
> + \
> for (i = 0; i < nels; i++) { \
> t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
> if (unlikely(stp##_is_signaling_nan(xb->sfld, \
> @@ -2787,6 +2795,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> ppc_vsr_t t = { }; \
> int i; \
> \
> + helper_reset_fpstatus(env); \
> + \
> for (i = 0; i < nels; i++) { \
> t.tfld = stp##_to_##ttp(xb->sfld, 1, &env->fp_status); \
> if (unlikely(stp##_is_signaling_nan(xb->sfld, \
> @@ -2834,6 +2844,8 @@ void helper_XSCVQPDP(CPUPPCState *env, uint32_t ro, ppc_vsr_t *xt,
> ppc_vsr_t t = { };
> float_status tstat;
>
> + helper_reset_fpstatus(env);
> +
> tstat = env->fp_status;
> if (ro != 0) {
> tstat.float_rounding_mode = float_round_to_odd;
> @@ -2855,6 +2867,7 @@ uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb)
> {
> uint64_t result, sign, exp, frac;
>
> + helper_reset_fpstatus(env);
> float_status tstat = env->fp_status;
> set_float_exception_flags(0, &tstat);
>
> @@ -2910,22 +2923,20 @@ uint64_t helper_XSCVSPDPN(uint64_t xb)
> #define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, sfi, rnan) \
> void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> { \
> - int all_flags = env->fp_status.float_exception_flags, flags; \
> ppc_vsr_t t = { }; \
> - int i; \
> + int i, flags; \
> + \
> + helper_reset_fpstatus(env); \
> \
> for (i = 0; i < nels; i++) { \
> - env->fp_status.float_exception_flags = 0; \
> t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status); \
> flags = env->fp_status.float_exception_flags; \
> if (unlikely(flags & float_flag_invalid)) { \
> t.tfld = float_invalid_cvt(env, flags, t.tfld, rnan, 0, GETPC());\
> } \
> - all_flags |= flags; \
> } \
> \
> *xt = t; \
> - env->fp_status.float_exception_flags = all_flags; \
> do_float_check_status(env, sfi, GETPC()); \
> }
>
> @@ -2977,12 +2988,12 @@ VSX_CVT_FP_TO_INT128(XSCVQPSQZ, int128, 0x8000000000000000ULL);
> #define VSX_CVT_FP_TO_INT2(op, nels, stp, ttp, sfi, rnan) \
> void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> { \
> - int all_flags = env->fp_status.float_exception_flags, flags; \
> ppc_vsr_t t = { }; \
> - int i; \
> + int i, flags; \
> + \
> + helper_reset_fpstatus(env); \
> \
> for (i = 0; i < nels; i++) { \
> - env->fp_status.float_exception_flags = 0; \
> t.VsrW(2 * i) = stp##_to_##ttp##_round_to_zero(xb->VsrD(i), \
> &env->fp_status); \
> flags = env->fp_status.float_exception_flags; \
> @@ -2991,11 +3002,9 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> rnan, 0, GETPC()); \
> } \
> t.VsrW(2 * i + 1) = t.VsrW(2 * i); \
> - all_flags |= flags; \
> } \
> \
> *xt = t; \
> - env->fp_status.float_exception_flags = all_flags; \
> do_float_check_status(env, sfi, GETPC()); \
> }
>
> @@ -3020,6 +3029,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
> ppc_vsr_t t = { }; \
> int flags; \
> \
> + helper_reset_fpstatus(env); \
> + \
> t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status); \
> flags = get_float_exception_flags(&env->fp_status); \
> if (flags & float_flag_invalid) { \
> @@ -3032,7 +3043,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
>
> VSX_CVT_FP_TO_INT_VECTOR(xscvqpsdz, float128, int64, f128, VsrD(0), \
> 0x8000000000000000ULL)
> -
> VSX_CVT_FP_TO_INT_VECTOR(xscvqpswz, float128, int32, f128, VsrD(0), \
> 0xffffffff80000000ULL)
> VSX_CVT_FP_TO_INT_VECTOR(xscvqpudz, float128, uint64, f128, VsrD(0), 0x0ULL)
> @@ -3055,6 +3065,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> ppc_vsr_t t = { }; \
> int i; \
> \
> + helper_reset_fpstatus(env); \
> + \
> for (i = 0; i < nels; i++) { \
> t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
> if (r2sp) { \
> @@ -3124,6 +3136,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \
> { \
> ppc_vsr_t t = *xt; \
> \
> + helper_reset_fpstatus(env); \
> t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status); \
> helper_compute_fprf_##ttp(env, t.tfld); \
> \
> @@ -3157,6 +3170,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb) \
> int i; \
> FloatRoundMode curr_rounding_mode; \
> \
> + helper_reset_fpstatus(env); \
> + \
> if (rmode != FLOAT_ROUND_CURRENT) { \
> curr_rounding_mode = get_float_rounding_mode(&env->fp_status); \
> set_float_rounding_mode(rmode, &env->fp_status); \
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/8] Multiple ppc instructions fixes
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
` (7 preceding siblings ...)
2022-09-06 12:55 ` [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it Víctor Colombo
@ 2022-09-06 20:07 ` Daniel Henrique Barboza
8 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2022-09-06 20:07 UTC (permalink / raw)
To: Víctor Colombo, qemu-devel, qemu-ppc
Cc: clg, david, groug, richard.henderson, matheus.ferst, lucas.araujo,
leandro.lupori, lucas.coutinho
Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,
Daniel
On 9/6/22 09:55, Víctor Colombo wrote:
> This patch set fixes multiple instructions for PPC targets that were
> producing incorrect results, or setting the wrong bits in FPSCR.
>
> Patch 1 is just a style fix, trivial.
> Patch 8 adds helper_reset_fpstatus() calls to instructions
> that have an issue where the exception flags are being kept from
> the previous instruction, causing incorrect bits to be set,
> specially the non-sticky FI bit.
> Other patches fixes other specific situations.
>
> v1->v2:
> - Squash patches 8 through 19 and write a better commit message to it.
> - Dropped Daniel's R-b in the squashed patches, as the squash merged
> both reviewed and non-reviewed patches. Now require a new, single
> R-b.
>
> Víctor Colombo (8):
> target/ppc: Remove extra space from s128 field in ppc_vsr_t
> target/ppc: Remove unused xer_* macros
> target/ppc: Zero second doubleword in DFP instructions
> target/ppc: Set result to QNaN for DENBCD when VXCVI occurs
> target/ppc: Zero second doubleword for VSX madd instructions
> target/ppc: Set OV32 when OV is set
> target/ppc: Zero second doubleword of VSR registers for FPR insns
> target/ppc: Clear fpstatus flags on helpers missing it
>
> target/ppc/cpu.h | 6 +-----
> target/ppc/dfp_helper.c | 31 ++++++++++++++++++++++++++++---
> target/ppc/fpu_helper.c | 39 +++++++++++++++++++++++++++------------
> target/ppc/int_helper.c | 4 ++--
> target/ppc/translate.c | 8 ++++++++
> 5 files changed, 66 insertions(+), 22 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-09-06 20:09 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-06 12:55 [PATCH v2 0/8] Multiple ppc instructions fixes Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 1/8] target/ppc: Remove extra space from s128 field in ppc_vsr_t Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 2/8] target/ppc: Remove unused xer_* macros Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 3/8] target/ppc: Zero second doubleword in DFP instructions Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 4/8] target/ppc: Set result to QNaN for DENBCD when VXCVI occurs Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 5/8] target/ppc: Zero second doubleword for VSX madd instructions Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 6/8] target/ppc: Set OV32 when OV is set Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 7/8] target/ppc: Zero second doubleword of VSR registers for FPR insns Víctor Colombo
2022-09-06 12:55 ` [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it Víctor Colombo
2022-09-06 17:16 ` Daniel Henrique Barboza
2022-09-06 20:07 ` [PATCH v2 0/8] Multiple ppc instructions fixes Daniel Henrique Barboza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).