* [PATCH 0/7] target/ppc: DFP fixes and improvements
@ 2019-09-24 15:35 Mark Cave-Ayland
2019-09-24 15:35 ` [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions Mark Cave-Ayland
` (7 more replies)
0 siblings, 8 replies; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
This patchset fixes the DFP issue reported at https://bugs.launchpad.net/qemu/+bug/1841990
caused by the change in FP register storage in commit ef96e3ae96 "target/ppc:
move FP and VMX registers into aligned vsr register array" along with some
further tidy-up/improvements.
Patches 1 and 2 introduce get/set helper functions for reading and writing
DFP even-odd register pairs (rather than accessing the register pointers
directly) which then leads to the real fix in patch 3.
Following on from this patches 4 to 6 change the struct PPC_DFP internal
decimal representation from uint64[2] to ppc_vsr_t which enables us to use
the existing VsrD() macro to access the correct elements regardless of host
endian and remove the explicit HI_IDX and LO_IDX references.
Finally patch 7 simplifies the calls to set_dfp{64,128}() in DFP macros
which can now be generated directly by the preprocessor rather than requiring
an explicit if() statement.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Mark Cave-Ayland (7):
target/ppc: introduce get_dfp{64,128}() helper functions
target/ppc: introduce set_dfp{64,128}() helper functions
target/ppc: update {get,set}_dfp{64,128}() helper functions to
read/write DFP numbers correctly
target/ppc: introduce dfp_finalize_decimal{64,128}() helper functions
target/ppc: change struct PPC_DFP decimal storage from uint64[2] to
ppc_vsr_t
target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX
from dfp_helper.c
target/ppc: remove unnecessary if() around calls to set_dfp{64,128}()
in DFP macros
target/ppc/cpu.h | 1 +
target/ppc/dfp_helper.c | 384 ++++++++++++++++++++--------------------
target/ppc/helper.h | 2 +-
3 files changed, 193 insertions(+), 194 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 19:21 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 2/7] target/ppc: introduce set_dfp{64,128}() " Mark Cave-Ayland
` (6 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
The existing functions (now incorrectly) assume that the MSB and LSB of DFP
numbers are stored as consecutive 64-bit words in memory. Instead of accessing
the DFP numbers directly, introduce get_dfp{64,128}() helper functions to ease
the switch to the correct representation.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/dfp_helper.c | 52 ++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index f102177572..354a4aa877 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -36,6 +36,17 @@
#define LO_IDX 0
#endif
+static void get_dfp64(uint64_t *dst, uint64_t *dfp)
+{
+ dst[0] = dfp[0];
+}
+
+static void get_dfp128(uint64_t *dst, uint64_t *dfp)
+{
+ dst[0] = dfp[HI_IDX];
+ dst[1] = dfp[LO_IDX];
+}
+
struct PPC_DFP {
CPUPPCState *env;
uint64_t t64[2], a64[2], b64[2];
@@ -129,7 +140,7 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, uint64_t *a,
dfp->env = env;
if (a) {
- dfp->a64[0] = *a;
+ get_dfp64(dfp->a64, a);
decimal64ToNumber((decimal64 *)dfp->a64, &dfp->a);
} else {
dfp->a64[0] = 0;
@@ -137,7 +148,7 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, uint64_t *a,
}
if (b) {
- dfp->b64[0] = *b;
+ get_dfp64(dfp->b64, b);
decimal64ToNumber((decimal64 *)dfp->b64, &dfp->b);
} else {
dfp->b64[0] = 0;
@@ -153,8 +164,7 @@ static void dfp_prepare_decimal128(struct PPC_DFP *dfp, uint64_t *a,
dfp->env = env;
if (a) {
- dfp->a64[0] = a[HI_IDX];
- dfp->a64[1] = a[LO_IDX];
+ get_dfp128(dfp->a64, a);
decimal128ToNumber((decimal128 *)dfp->a64, &dfp->a);
} else {
dfp->a64[0] = dfp->a64[1] = 0;
@@ -162,8 +172,7 @@ static void dfp_prepare_decimal128(struct PPC_DFP *dfp, uint64_t *a,
}
if (b) {
- dfp->b64[0] = b[HI_IDX];
- dfp->b64[1] = b[LO_IDX];
+ get_dfp128(dfp->b64, b);
decimal128ToNumber((decimal128 *)dfp->b64, &dfp->b);
} else {
dfp->b64[0] = dfp->b64[1] = 0;
@@ -617,10 +626,12 @@ uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint64_t *b) \
{ \
struct PPC_DFP dfp; \
unsigned k; \
+ uint64_t a64; \
\
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
- k = *a & 0x3F; \
+ get_dfp64(&a64, a); \
+ k = a64 & 0x3F; \
\
if (unlikely(decNumberIsSpecial(&dfp.b))) { \
dfp.crbf = 1; \
@@ -817,11 +828,15 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
uint64_t *b, uint32_t rmc) \
{ \
struct PPC_DFP dfp; \
- int32_t ref_sig = *a & 0x3F; \
+ uint64_t a64; \
+ int32_t ref_sig; \
int32_t xmax = ((size) == 64) ? 369 : 6111; \
\
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
+ get_dfp64(&a64, a); \
+ ref_sig = a64 & 0x3f; \
+ \
_dfp_reround(rmc, ref_sig, xmax, &dfp); \
decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
&dfp.context); \
@@ -881,7 +896,12 @@ DFP_HELPER_RINT(drintnq, RINTN_PPs, 128)
void helper_dctdp(CPUPPCState *env, uint64_t *t, uint64_t *b)
{
struct PPC_DFP dfp;
- uint32_t b_short = *b;
+ uint64_t b64;
+ uint32_t b_short;
+
+ get_dfp64(&b64, b);
+ b_short = (uint32_t)b64;
+
dfp_prepare_decimal64(&dfp, 0, 0, env);
decimal32ToNumber((decimal32 *)&b_short, &dfp.t);
decimal64FromNumber((decimal64 *)t, &dfp.t, &dfp.context);
@@ -891,8 +911,10 @@ void helper_dctdp(CPUPPCState *env, uint64_t *t, uint64_t *b)
void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
{
struct PPC_DFP dfp;
+ uint64_t b64;
dfp_prepare_decimal128(&dfp, 0, 0, env);
- decimal64ToNumber((decimal64 *)b, &dfp.t);
+ get_dfp64(&b64, b);
+ decimal64ToNumber((decimal64 *)&b64, &dfp.t);
dfp_check_for_VXSNAN_and_convert_to_QNaN(&dfp);
dfp_set_FPRF_from_FRT(&dfp);
@@ -940,8 +962,10 @@ void helper_drdpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
{ \
struct PPC_DFP dfp; \
+ uint64_t b64; \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
- decNumberFromInt64(&dfp.t, (int64_t)(*b)); \
+ get_dfp64(&b64, b); \
+ decNumberFromInt64(&dfp.t, (int64_t)b64); \
decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context); \
CFFIX_PPs(&dfp); \
\
@@ -1183,10 +1207,12 @@ static void dfp_set_raw_exp_128(uint64_t *t, uint64_t raw)
void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, uint64_t *b) \
{ \
struct PPC_DFP dfp; \
- uint64_t raw_qnan, raw_snan, raw_inf, max_exp; \
+ uint64_t raw_qnan, raw_snan, raw_inf, max_exp, a64; \
int bias; \
- int64_t exp = *((int64_t *)a); \
+ int64_t exp; \
\
+ get_dfp64(&a64, a); \
+ exp = (int64_t)a64; \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
if ((size) == 64) { \
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/7] target/ppc: introduce set_dfp{64,128}() helper functions
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
2019-09-24 15:35 ` [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 21:27 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 3/7] target/ppc: update {get, set}_dfp{64, 128}() helper functions to read/write DFP numbers correctly Mark Cave-Ayland
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
The existing functions (now incorrectly) assume that the MSB and LSB of DFP
numbers are stored as consecutive 64-bit words in memory. Instead of accessing
the DFP numbers directly, introduce set_dfp{64,128}() helper functions to ease
the switch to the correct representation.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/dfp_helper.c | 90 ++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index 354a4aa877..cb98780d20 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -47,6 +47,17 @@ static void get_dfp128(uint64_t *dst, uint64_t *dfp)
dst[1] = dfp[LO_IDX];
}
+static void set_dfp64(uint64_t *dfp, uint64_t *src)
+{
+ dfp[0] = src[0];
+}
+
+static void set_dfp128(uint64_t *dfp, uint64_t *src)
+{
+ dfp[0] = src[HI_IDX];
+ dfp[1] = src[LO_IDX];
+}
+
struct PPC_DFP {
CPUPPCState *env;
uint64_t t64[2], a64[2], b64[2];
@@ -413,10 +424,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, uint64_t *b) \
decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context); \
postprocs(&dfp); \
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -735,10 +745,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, \
QUA_PPs(&dfp); \
\
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -759,10 +768,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
QUA_PPs(&dfp); \
\
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -843,10 +851,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
QUA_PPs(&dfp); \
\
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -867,10 +874,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, \
postprocs(&dfp); \
\
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -904,7 +910,8 @@ void helper_dctdp(CPUPPCState *env, uint64_t *t, uint64_t *b)
dfp_prepare_decimal64(&dfp, 0, 0, env);
decimal32ToNumber((decimal32 *)&b_short, &dfp.t);
- decimal64FromNumber((decimal64 *)t, &dfp.t, &dfp.context);
+ decimal64FromNumber((decimal64 *)&dfp.t64, &dfp.t, &dfp.context);
+ set_dfp64(t, dfp.t64);
dfp_set_FPRF_from_FRT(&dfp);
}
@@ -920,14 +927,14 @@ void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
dfp_set_FPRF_from_FRT(&dfp);
decimal128FromNumber((decimal128 *)&dfp.t64, &dfp.t, &dfp.context);
- t[0] = dfp.t64[HI_IDX];
- t[1] = dfp.t64[LO_IDX];
+ set_dfp128(t, dfp.t64);
}
void helper_drsp(CPUPPCState *env, uint64_t *t, uint64_t *b)
{
struct PPC_DFP dfp;
uint32_t t_short = 0;
+ uint64_t t64;
dfp_prepare_decimal64(&dfp, 0, b, env);
decimal32FromNumber((decimal32 *)&t_short, &dfp.b, &dfp.context);
decimal32ToNumber((decimal32 *)&t_short, &dfp.t);
@@ -937,7 +944,8 @@ void helper_drsp(CPUPPCState *env, uint64_t *t, uint64_t *b)
dfp_check_for_UX(&dfp);
dfp_check_for_XX(&dfp);
- *t = t_short;
+ t64 = (uint64_t)t_short;
+ set_dfp64(t, &t64);
}
void helper_drdpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
@@ -953,9 +961,9 @@ void helper_drdpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
dfp_check_for_UX(&dfp);
dfp_check_for_XX(&dfp);
+ dfp.t64[0] = dfp.t64[1] = 0;
decimal64FromNumber((decimal64 *)dfp.t64, &dfp.t, &dfp.context);
- t[0] = dfp.t64[0];
- t[1] = 0;
+ set_dfp128(t, dfp.t64);
}
#define DFP_HELPER_CFFIX(op, size) \
@@ -970,10 +978,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
CFFIX_PPs(&dfp); \
\
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -1016,7 +1023,7 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
} \
} \
\
- *t = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
}
DFP_HELPER_CTFIX(dctfix, 64)
@@ -1078,10 +1085,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, uint32_t sp) \
} \
\
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -1150,10 +1156,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, uint32_t s) \
&dfp.context); \
dfp_set_FPRF_from_FRT(&dfp); \
if ((size) == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if ((size) == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -1164,27 +1169,30 @@ DFP_HELPER_ENBCD(denbcdq, 128)
void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
{ \
struct PPC_DFP dfp; \
+ uint64_t t64; \
\
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
if (unlikely(decNumberIsSpecial(&dfp.b))) { \
if (decNumberIsInfinite(&dfp.b)) { \
- *t = -1; \
+ t64 = -1; \
} else if (decNumberIsSNaN(&dfp.b)) { \
- *t = -3; \
+ t64 = -3; \
} else if (decNumberIsQNaN(&dfp.b)) { \
- *t = -2; \
+ t64 = -2; \
} else { \
assert(0); \
} \
+ set_dfp64(t, &t64); \
} else { \
if ((size) == 64) { \
- *t = dfp.b.exponent + 398; \
+ t64 = dfp.b.exponent + 398; \
} else if ((size) == 128) { \
- *t = dfp.b.exponent + 6176; \
+ t64 = dfp.b.exponent + 6176; \
} else { \
assert(0); \
} \
+ set_dfp64(t, &t64); \
} \
}
@@ -1251,10 +1259,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, uint64_t *b) \
&dfp.context); \
} \
if (size == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else if (size == 128) { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
@@ -1344,10 +1351,9 @@ void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
} \
\
if ((size) == 64) { \
- t[0] = dfp.t64[0]; \
+ set_dfp64(t, dfp.t64); \
} else { \
- t[0] = dfp.t64[HI_IDX]; \
- t[1] = dfp.t64[LO_IDX]; \
+ set_dfp128(t, dfp.t64); \
} \
}
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/7] target/ppc: update {get, set}_dfp{64, 128}() helper functions to read/write DFP numbers correctly
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
2019-09-24 15:35 ` [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions Mark Cave-Ayland
2019-09-24 15:35 ` [PATCH 2/7] target/ppc: introduce set_dfp{64,128}() " Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 21:33 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 4/7] target/ppc: introduce dfp_finalize_decimal{64, 128}() helper functions Mark Cave-Ayland
` (4 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
Since commit ef96e3ae96 "target/ppc: move FP and VMX registers into aligned vsr
register array" FP registers are no longer stored consecutively in memory and so
the current method of combining FP register pairs into DFP numbers is incorrect.
Firstly update the definition of the dh_*_fprp defines in helper.h to reflect
that FP registers are now stored as part of an array of ppc_vsr_t elements
rather than plain uint64_t elements, and then introduce a new ppc_fprp_t type
which conceptually represents a DFP even-odd register pair to be consumed by the
DFP helper functions.
Finally update the new DFP {get,set}_dfp{64,128}() helper functions to convert
between DFP numbers and DFP even-odd register pairs correctly, making use of the
existing VsrD() macro to access the correct elements regardless of host endian.
Fixes: ef96e3ae96 "target/ppc: move FP and VMX registers into aligned vsr register array"
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/cpu.h | 1 +
target/ppc/dfp_helper.c | 80 +++++++++++++++++++++--------------------
target/ppc/helper.h | 2 +-
3 files changed, 44 insertions(+), 39 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index eaee1a5575..b79bbc171f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -235,6 +235,7 @@ typedef union _ppc_vsr_t {
} ppc_vsr_t;
typedef ppc_vsr_t ppc_avr_t;
+typedef ppc_vsr_t ppc_fprp_t;
#if !defined(CONFIG_USER_ONLY)
/* Software TLB cache */
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index cb98780d20..df9026ea5e 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -36,26 +36,26 @@
#define LO_IDX 0
#endif
-static void get_dfp64(uint64_t *dst, uint64_t *dfp)
+static void get_dfp64(uint64_t *dst, ppc_fprp_t *dfp)
{
- dst[0] = dfp[0];
+ dst[0] = dfp->VsrD(0);
}
-static void get_dfp128(uint64_t *dst, uint64_t *dfp)
+static void get_dfp128(uint64_t *dst, ppc_fprp_t *dfp)
{
- dst[0] = dfp[HI_IDX];
- dst[1] = dfp[LO_IDX];
+ dst[HI_IDX] = dfp[0].VsrD(0);
+ dst[LO_IDX] = dfp[1].VsrD(0);
}
-static void set_dfp64(uint64_t *dfp, uint64_t *src)
+static void set_dfp64(ppc_fprp_t *dfp, uint64_t *src)
{
- dfp[0] = src[0];
+ dfp->VsrD(0) = src[0];
}
-static void set_dfp128(uint64_t *dfp, uint64_t *src)
+static void set_dfp128(ppc_fprp_t *dfp, uint64_t *src)
{
- dfp[0] = src[HI_IDX];
- dfp[1] = src[LO_IDX];
+ dfp[0].VsrD(0) = src[HI_IDX];
+ dfp[1].VsrD(0) = src[LO_IDX];
}
struct PPC_DFP {
@@ -143,8 +143,8 @@ static void dfp_set_round_mode_from_immediate(uint8_t r, uint8_t rmc,
decContextSetRounding(&dfp->context, rnd);
}
-static void dfp_prepare_decimal64(struct PPC_DFP *dfp, uint64_t *a,
- uint64_t *b, CPUPPCState *env)
+static void dfp_prepare_decimal64(struct PPC_DFP *dfp, ppc_fprp_t *a,
+ ppc_fprp_t *b, CPUPPCState *env)
{
decContextDefault(&dfp->context, DEC_INIT_DECIMAL64);
dfp_prepare_rounding_mode(&dfp->context, env->fpscr);
@@ -167,8 +167,8 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, uint64_t *a,
}
}
-static void dfp_prepare_decimal128(struct PPC_DFP *dfp, uint64_t *a,
- uint64_t *b, CPUPPCState *env)
+static void dfp_prepare_decimal128(struct PPC_DFP *dfp, ppc_fprp_t *a,
+ ppc_fprp_t *b, CPUPPCState *env)
{
decContextDefault(&dfp->context, DEC_INIT_DECIMAL128);
dfp_prepare_rounding_mode(&dfp->context, env->fpscr);
@@ -416,7 +416,8 @@ static inline int dfp_get_digit(decNumber *dn, int n)
}
#define DFP_HELPER_TAB(op, dnop, postprocs, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, uint64_t *b) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
+ ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
dfp_prepare_decimal##size(&dfp, a, b, env); \
@@ -485,7 +486,7 @@ DFP_HELPER_TAB(ddiv, decNumberDivide, DIV_PPs, 64)
DFP_HELPER_TAB(ddivq, decNumberDivide, DIV_PPs, 128)
#define DFP_HELPER_BF_AB(op, dnop, postprocs, size) \
-uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint64_t *b) \
+uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
dfp_prepare_decimal##size(&dfp, a, b, env); \
@@ -517,7 +518,7 @@ DFP_HELPER_BF_AB(dcmpo, decNumberCompare, CMPO_PPs, 64)
DFP_HELPER_BF_AB(dcmpoq, decNumberCompare, CMPO_PPs, 128)
#define DFP_HELPER_TSTDC(op, size) \
-uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint32_t dcm) \
+uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, uint32_t dcm) \
{ \
struct PPC_DFP dfp; \
int match = 0; \
@@ -545,7 +546,7 @@ DFP_HELPER_TSTDC(dtstdc, 64)
DFP_HELPER_TSTDC(dtstdcq, 128)
#define DFP_HELPER_TSTDG(op, size) \
-uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint32_t dcm) \
+uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, uint32_t dcm) \
{ \
struct PPC_DFP dfp; \
int minexp, maxexp, nzero_digits, nzero_idx, is_negative, is_zero, \
@@ -600,7 +601,7 @@ DFP_HELPER_TSTDG(dtstdg, 64)
DFP_HELPER_TSTDG(dtstdgq, 128)
#define DFP_HELPER_TSTEX(op, size) \
-uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint64_t *b) \
+uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
int expa, expb, a_is_special, b_is_special; \
@@ -632,7 +633,7 @@ DFP_HELPER_TSTEX(dtstex, 64)
DFP_HELPER_TSTEX(dtstexq, 128)
#define DFP_HELPER_TSTSF(op, size) \
-uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint64_t *b) \
+uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
unsigned k; \
@@ -669,7 +670,7 @@ DFP_HELPER_TSTSF(dtstsf, 64)
DFP_HELPER_TSTSF(dtstsfq, 128)
#define DFP_HELPER_TSTSFI(op, size) \
-uint32_t helper_##op(CPUPPCState *env, uint32_t a, uint64_t *b) \
+uint32_t helper_##op(CPUPPCState *env, uint32_t a, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
unsigned uim; \
@@ -729,7 +730,7 @@ static void dfp_quantize(uint8_t rmc, struct PPC_DFP *dfp)
}
#define DFP_HELPER_QUAI(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
uint32_t te, uint32_t rmc) \
{ \
struct PPC_DFP dfp; \
@@ -755,8 +756,8 @@ DFP_HELPER_QUAI(dquai, 64)
DFP_HELPER_QUAI(dquaiq, 128)
#define DFP_HELPER_QUA(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
- uint64_t *b, uint32_t rmc) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
+ ppc_fprp_t *b, uint32_t rmc) \
{ \
struct PPC_DFP dfp; \
\
@@ -832,8 +833,8 @@ static void _dfp_reround(uint8_t rmc, int32_t ref_sig, int32_t xmax,
}
#define DFP_HELPER_RRND(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
- uint64_t *b, uint32_t rmc) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
+ ppc_fprp_t *b, uint32_t rmc) \
{ \
struct PPC_DFP dfp; \
uint64_t a64; \
@@ -861,7 +862,7 @@ DFP_HELPER_RRND(drrnd, 64)
DFP_HELPER_RRND(drrndq, 128)
#define DFP_HELPER_RINT(op, postprocs, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
uint32_t r, uint32_t rmc) \
{ \
struct PPC_DFP dfp; \
@@ -899,7 +900,7 @@ static void RINTN_PPs(struct PPC_DFP *dfp)
DFP_HELPER_RINT(drintn, RINTN_PPs, 64)
DFP_HELPER_RINT(drintnq, RINTN_PPs, 128)
-void helper_dctdp(CPUPPCState *env, uint64_t *t, uint64_t *b)
+void helper_dctdp(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
uint64_t b64;
@@ -915,7 +916,7 @@ void helper_dctdp(CPUPPCState *env, uint64_t *t, uint64_t *b)
dfp_set_FPRF_from_FRT(&dfp);
}
-void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
+void helper_dctqpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
uint64_t b64;
@@ -930,7 +931,7 @@ void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
set_dfp128(t, dfp.t64);
}
-void helper_drsp(CPUPPCState *env, uint64_t *t, uint64_t *b)
+void helper_drsp(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
uint32_t t_short = 0;
@@ -948,7 +949,7 @@ void helper_drsp(CPUPPCState *env, uint64_t *t, uint64_t *b)
set_dfp64(t, &t64);
}
-void helper_drdpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
+void helper_drdpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
dfp_prepare_decimal128(&dfp, 0, b, env);
@@ -967,7 +968,7 @@ void helper_drdpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
}
#define DFP_HELPER_CFFIX(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
uint64_t b64; \
@@ -994,7 +995,7 @@ DFP_HELPER_CFFIX(dcffix, 64)
DFP_HELPER_CFFIX(dcffixq, 128)
#define DFP_HELPER_CTFIX(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
@@ -1057,7 +1058,8 @@ static inline void dfp_set_sign_128(uint64_t *t, uint8_t sgn)
}
#define DFP_HELPER_DEDPD(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, uint32_t sp) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
+ uint32_t sp) \
{ \
struct PPC_DFP dfp; \
uint8_t digits[34]; \
@@ -1105,7 +1107,8 @@ static inline uint8_t dfp_get_bcd_digit_128(uint64_t *t, unsigned n)
}
#define DFP_HELPER_ENBCD(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b, uint32_t s) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
+ uint32_t s) \
{ \
struct PPC_DFP dfp; \
uint8_t digits[32]; \
@@ -1166,7 +1169,7 @@ DFP_HELPER_ENBCD(denbcd, 64)
DFP_HELPER_ENBCD(denbcdq, 128)
#define DFP_HELPER_XEX(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *b) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
uint64_t t64; \
@@ -1212,7 +1215,8 @@ static void dfp_set_raw_exp_128(uint64_t *t, uint64_t raw)
}
#define DFP_HELPER_IEX(op, size) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, uint64_t *b) \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
+ ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
uint64_t raw_qnan, raw_snan, raw_inf, max_exp, a64; \
@@ -1309,7 +1313,7 @@ static void dfp_clear_lmd_from_g5msb(uint64_t *t)
}
#define DFP_HELPER_SHIFT(op, size, shift_left) \
-void helper_##op(CPUPPCState *env, uint64_t *t, uint64_t *a, \
+void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
uint32_t sh) \
{ \
struct PPC_DFP dfp; \
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 54ea9b9500..f843814b8a 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -686,7 +686,7 @@ DEF_HELPER_3(store_601_batu, void, env, i32, tl)
#endif
#define dh_alias_fprp ptr
-#define dh_ctype_fprp uint64_t *
+#define dh_ctype_fprp ppc_fprp_t *
#define dh_is_signed_fprp dh_is_signed_ptr
DEF_HELPER_4(dadd, void, env, fprp, fprp, fprp)
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/7] target/ppc: introduce dfp_finalize_decimal{64, 128}() helper functions
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
` (2 preceding siblings ...)
2019-09-24 15:35 ` [PATCH 3/7] target/ppc: update {get, set}_dfp{64, 128}() helper functions to read/write DFP numbers correctly Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 21:47 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t Mark Cave-Ayland
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
Most of the DFP helper functions call decimal{64,128}FromNumber() just before
returning in order to convert the decNumber stored in dfp.t64 back to a
Decimal{64,128} to write back to the FP registers.
Introduce new dfp_finalize_decimal{64,128}() helper functions which both enable
the parameter list to be reduced considerably, and also help minimise the
changes required in the next patch.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/dfp_helper.c | 42 ++++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index df9026ea5e..52b123b16a 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -191,6 +191,16 @@ static void dfp_prepare_decimal128(struct PPC_DFP *dfp, ppc_fprp_t *a,
}
}
+static void dfp_finalize_decimal64(struct PPC_DFP *dfp)
+{
+ decimal64FromNumber((decimal64 *)&dfp->t64, &dfp->t, &dfp->context);
+}
+
+static void dfp_finalize_decimal128(struct PPC_DFP *dfp)
+{
+ decimal128FromNumber((decimal128 *)&dfp->t64, &dfp->t, &dfp->context);
+}
+
static void dfp_set_FPSCR_flag(struct PPC_DFP *dfp, uint64_t flag,
uint64_t enabled)
{
@@ -422,7 +432,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
struct PPC_DFP dfp; \
dfp_prepare_decimal##size(&dfp, a, b, env); \
dnop(&dfp.t, &dfp.a, &dfp.b, &dfp.context); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
postprocs(&dfp); \
if (size == 64) { \
set_dfp64(t, dfp.t64); \
@@ -491,7 +501,7 @@ uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, ppc_fprp_t *b) \
struct PPC_DFP dfp; \
dfp_prepare_decimal##size(&dfp, a, b, env); \
dnop(&dfp.t, &dfp.a, &dfp.b, &dfp.context); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
postprocs(&dfp); \
return dfp.crbf; \
}
@@ -741,8 +751,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
dfp.a.exponent = (int32_t)((int8_t)(te << 3) >> 3); \
\
dfp_quantize(rmc, &dfp); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
- &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
if (size == 64) { \
@@ -764,8 +773,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp_prepare_decimal##size(&dfp, a, b, env); \
\
dfp_quantize(rmc, &dfp); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
- &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
if (size == 64) { \
@@ -847,8 +855,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
ref_sig = a64 & 0x3f; \
\
_dfp_reround(rmc, ref_sig, xmax, &dfp); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
- &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
if (size == 64) { \
@@ -871,7 +878,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
\
dfp_set_round_mode_from_immediate(r, rmc, &dfp); \
decNumberToIntegralExact(&dfp.t, &dfp.b, &dfp.context); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
postprocs(&dfp); \
\
if (size == 64) { \
@@ -911,7 +918,7 @@ void helper_dctdp(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
dfp_prepare_decimal64(&dfp, 0, 0, env);
decimal32ToNumber((decimal32 *)&b_short, &dfp.t);
- decimal64FromNumber((decimal64 *)&dfp.t64, &dfp.t, &dfp.context);
+ dfp_finalize_decimal64(&dfp);
set_dfp64(t, dfp.t64);
dfp_set_FPRF_from_FRT(&dfp);
}
@@ -927,7 +934,7 @@ void helper_dctqpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
dfp_check_for_VXSNAN_and_convert_to_QNaN(&dfp);
dfp_set_FPRF_from_FRT(&dfp);
- decimal128FromNumber((decimal128 *)&dfp.t64, &dfp.t, &dfp.context);
+ dfp_finalize_decimal128(&dfp);
set_dfp128(t, dfp.t64);
}
@@ -963,7 +970,7 @@ void helper_drdpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
dfp_check_for_XX(&dfp);
dfp.t64[0] = dfp.t64[1] = 0;
- decimal64FromNumber((decimal64 *)dfp.t64, &dfp.t, &dfp.context);
+ dfp_finalize_decimal64(&dfp);
set_dfp128(t, dfp.t64);
}
@@ -975,7 +982,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
get_dfp64(&b64, b); \
decNumberFromInt64(&dfp.t, (int64_t)b64); \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
CFFIX_PPs(&dfp); \
\
if (size == 64) { \
@@ -1155,8 +1162,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
if (s && sgn) { \
dfp.t.bits |= DECNEG; \
} \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
- &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
dfp_set_FPRF_from_FRT(&dfp); \
if ((size) == 64) { \
set_dfp64(t, dfp.t64); \
@@ -1259,8 +1265,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp.t.bits &= ~DECSPECIAL; \
} \
dfp.t.exponent = exp - bias; \
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
- &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
} \
if (size == 64) { \
set_dfp64(t, dfp.t64); \
@@ -1340,8 +1345,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp.t.digits = max_digits - 1; \
} \
\
- decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, \
- &dfp.context); \
+ dfp_finalize_decimal##size(&dfp); \
} else { \
if ((size) == 64) { \
dfp.t64[0] = dfp.a64[0] & 0xFFFC000000000000ULL; \
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
` (3 preceding siblings ...)
2019-09-24 15:35 ` [PATCH 4/7] target/ppc: introduce dfp_finalize_decimal{64, 128}() helper functions Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 21:41 ` Richard Henderson
2019-09-24 21:46 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c Mark Cave-Ayland
` (2 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
There are several places in dfp_helper.c that access the decimal number
representations in struct PPC_DFP via HI_IDX and LO_IDX defines which are set
at the top of dfp_helper.c according to the host endian.
However we can instead switch to using ppc_vsr_t for decimal numbers and then
make subsequent use of the existing VsrD() macros to access the correct
element regardless of host endian. Note that 64-bit decimals are stored in the
LSB of ppc_vsr_t (equivalent to VsrD(1)).
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/dfp_helper.c | 210 +++++++++++++++++++++-------------------
1 file changed, 108 insertions(+), 102 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index 52b123b16a..ed437f97da 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -36,31 +36,31 @@
#define LO_IDX 0
#endif
-static void get_dfp64(uint64_t *dst, ppc_fprp_t *dfp)
+static void get_dfp64(ppc_vsr_t *dst, ppc_fprp_t *dfp)
{
- dst[0] = dfp->VsrD(0);
+ dst->VsrD(1) = dfp->VsrD(0);
}
-static void get_dfp128(uint64_t *dst, ppc_fprp_t *dfp)
+static void get_dfp128(ppc_vsr_t *dst, ppc_fprp_t *dfp)
{
- dst[HI_IDX] = dfp[0].VsrD(0);
- dst[LO_IDX] = dfp[1].VsrD(0);
+ dst->VsrD(0) = dfp[0].VsrD(0);
+ dst->VsrD(1) = dfp[1].VsrD(0);
}
-static void set_dfp64(ppc_fprp_t *dfp, uint64_t *src)
+static void set_dfp64(ppc_fprp_t *dfp, ppc_vsr_t *src)
{
- dfp->VsrD(0) = src[0];
+ dfp->VsrD(0) = src->VsrD(1);
}
-static void set_dfp128(ppc_fprp_t *dfp, uint64_t *src)
+static void set_dfp128(ppc_fprp_t *dfp, ppc_vsr_t *src)
{
- dfp[0].VsrD(0) = src[HI_IDX];
- dfp[1].VsrD(0) = src[LO_IDX];
+ dfp[0].VsrD(0) = src->VsrD(0);
+ dfp[1].VsrD(0) = src->VsrD(1);
}
struct PPC_DFP {
CPUPPCState *env;
- uint64_t t64[2], a64[2], b64[2];
+ ppc_vsr_t vt, va, vb;
decNumber t, a, b;
decContext context;
uint8_t crbf;
@@ -151,18 +151,18 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, ppc_fprp_t *a,
dfp->env = env;
if (a) {
- get_dfp64(dfp->a64, a);
- decimal64ToNumber((decimal64 *)dfp->a64, &dfp->a);
+ get_dfp64(&dfp->va, a);
+ decimal64ToNumber((decimal64 *)&dfp->va.VsrD(1), &dfp->a);
} else {
- dfp->a64[0] = 0;
+ dfp->va.VsrD(1) = 0;
decNumberZero(&dfp->a);
}
if (b) {
- get_dfp64(dfp->b64, b);
- decimal64ToNumber((decimal64 *)dfp->b64, &dfp->b);
+ get_dfp64(&dfp->vb, b);
+ decimal64ToNumber((decimal64 *)&dfp->vb.VsrD(1), &dfp->b);
} else {
- dfp->b64[0] = 0;
+ dfp->vb.VsrD(1) = 0;
decNumberZero(&dfp->b);
}
}
@@ -175,30 +175,30 @@ static void dfp_prepare_decimal128(struct PPC_DFP *dfp, ppc_fprp_t *a,
dfp->env = env;
if (a) {
- get_dfp128(dfp->a64, a);
- decimal128ToNumber((decimal128 *)dfp->a64, &dfp->a);
+ get_dfp128(&dfp->va, a);
+ decimal128ToNumber((decimal128 *)&dfp->va, &dfp->a);
} else {
- dfp->a64[0] = dfp->a64[1] = 0;
+ dfp->va.VsrD(0) = dfp->va.VsrD(1) = 0;
decNumberZero(&dfp->a);
}
if (b) {
- get_dfp128(dfp->b64, b);
- decimal128ToNumber((decimal128 *)dfp->b64, &dfp->b);
+ get_dfp128(&dfp->vb, b);
+ decimal128ToNumber((decimal128 *)&dfp->vb, &dfp->b);
} else {
- dfp->b64[0] = dfp->b64[1] = 0;
+ dfp->vb.VsrD(0) = dfp->vb.VsrD(1) = 0;
decNumberZero(&dfp->b);
}
}
static void dfp_finalize_decimal64(struct PPC_DFP *dfp)
{
- decimal64FromNumber((decimal64 *)&dfp->t64, &dfp->t, &dfp->context);
+ decimal64FromNumber((decimal64 *)&dfp->vt.VsrD(1), &dfp->t, &dfp->context);
}
static void dfp_finalize_decimal128(struct PPC_DFP *dfp)
{
- decimal128FromNumber((decimal128 *)&dfp->t64, &dfp->t, &dfp->context);
+ decimal128FromNumber((decimal128 *)&dfp->vt, &dfp->t, &dfp->context);
}
static void dfp_set_FPSCR_flag(struct PPC_DFP *dfp, uint64_t flag,
@@ -435,9 +435,9 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp_finalize_decimal##size(&dfp); \
postprocs(&dfp); \
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -647,12 +647,12 @@ uint32_t helper_##op(CPUPPCState *env, ppc_fprp_t *a, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
unsigned k; \
- uint64_t a64; \
+ ppc_vsr_t va; \
\
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
- get_dfp64(&a64, a); \
- k = a64 & 0x3F; \
+ get_dfp64(&va, a); \
+ k = va.VsrD(1) & 0x3F; \
\
if (unlikely(decNumberIsSpecial(&dfp.b))) { \
dfp.crbf = 1; \
@@ -755,9 +755,9 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
QUA_PPs(&dfp); \
\
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -777,9 +777,9 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
QUA_PPs(&dfp); \
\
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -845,23 +845,23 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
ppc_fprp_t *b, uint32_t rmc) \
{ \
struct PPC_DFP dfp; \
- uint64_t a64; \
+ ppc_vsr_t va; \
int32_t ref_sig; \
int32_t xmax = ((size) == 64) ? 369 : 6111; \
\
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
- get_dfp64(&a64, a); \
- ref_sig = a64 & 0x3f; \
+ get_dfp64(&va, a); \
+ ref_sig = va.VsrD(1) & 0x3f; \
\
_dfp_reround(rmc, ref_sig, xmax, &dfp); \
dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -882,9 +882,9 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
postprocs(&dfp); \
\
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -910,39 +910,39 @@ DFP_HELPER_RINT(drintnq, RINTN_PPs, 128)
void helper_dctdp(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
- uint64_t b64;
+ ppc_vsr_t vb;
uint32_t b_short;
- get_dfp64(&b64, b);
- b_short = (uint32_t)b64;
+ get_dfp64(&vb, b);
+ b_short = (uint32_t)vb.VsrD(1);
dfp_prepare_decimal64(&dfp, 0, 0, env);
decimal32ToNumber((decimal32 *)&b_short, &dfp.t);
dfp_finalize_decimal64(&dfp);
- set_dfp64(t, dfp.t64);
+ set_dfp64(t, &dfp.vt);
dfp_set_FPRF_from_FRT(&dfp);
}
void helper_dctqpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
- uint64_t b64;
+ ppc_vsr_t vb;
dfp_prepare_decimal128(&dfp, 0, 0, env);
- get_dfp64(&b64, b);
- decimal64ToNumber((decimal64 *)&b64, &dfp.t);
+ get_dfp64(&vb, b);
+ decimal64ToNumber((decimal64 *)&vb.VsrD(1), &dfp.t);
dfp_check_for_VXSNAN_and_convert_to_QNaN(&dfp);
dfp_set_FPRF_from_FRT(&dfp);
dfp_finalize_decimal128(&dfp);
- set_dfp128(t, dfp.t64);
+ set_dfp128(t, &dfp.vt);
}
void helper_drsp(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
uint32_t t_short = 0;
- uint64_t t64;
+ ppc_vsr_t vt;
dfp_prepare_decimal64(&dfp, 0, b, env);
decimal32FromNumber((decimal32 *)&t_short, &dfp.b, &dfp.context);
decimal32ToNumber((decimal32 *)&t_short, &dfp.t);
@@ -952,16 +952,16 @@ void helper_drsp(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
dfp_check_for_UX(&dfp);
dfp_check_for_XX(&dfp);
- t64 = (uint64_t)t_short;
- set_dfp64(t, &t64);
+ vt.VsrD(1) = (uint64_t)t_short;
+ set_dfp64(t, &vt);
}
void helper_drdpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
{
struct PPC_DFP dfp;
dfp_prepare_decimal128(&dfp, 0, b, env);
- decimal64FromNumber((decimal64 *)&dfp.t64, &dfp.b, &dfp.context);
- decimal64ToNumber((decimal64 *)&dfp.t64, &dfp.t);
+ decimal64FromNumber((decimal64 *)&dfp.vt.VsrD(1), &dfp.b, &dfp.context);
+ decimal64ToNumber((decimal64 *)&dfp.vt.VsrD(1), &dfp.t);
dfp_check_for_VXSNAN_and_convert_to_QNaN(&dfp);
dfp_set_FPRF_from_FRT_long(&dfp);
@@ -969,26 +969,26 @@ void helper_drdpq(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b)
dfp_check_for_UX(&dfp);
dfp_check_for_XX(&dfp);
- dfp.t64[0] = dfp.t64[1] = 0;
+ dfp.vt.VsrD(0) = dfp.vt.VsrD(1) = 0;
dfp_finalize_decimal64(&dfp);
- set_dfp128(t, dfp.t64);
+ set_dfp128(t, &dfp.vt);
}
#define DFP_HELPER_CFFIX(op, size) \
void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
- uint64_t b64; \
+ ppc_vsr_t vb; \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
- get_dfp64(&b64, b); \
- decNumberFromInt64(&dfp.t, (int64_t)b64); \
+ get_dfp64(&vb, b); \
+ decNumberFromInt64(&dfp.t, (int64_t)vb.VsrD(1)); \
dfp_finalize_decimal##size(&dfp); \
CFFIX_PPs(&dfp); \
\
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -1010,28 +1010,30 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
if (unlikely(decNumberIsSpecial(&dfp.b))) { \
uint64_t invalid_flags = FP_VX | FP_VXCVI; \
if (decNumberIsInfinite(&dfp.b)) { \
- dfp.t64[0] = decNumberIsNegative(&dfp.b) ? INT64_MIN : INT64_MAX; \
+ dfp.vt.VsrD(1) = decNumberIsNegative(&dfp.b) ? INT64_MIN : \
+ INT64_MAX; \
} else { /* NaN */ \
- dfp.t64[0] = INT64_MIN; \
+ dfp.vt.VsrD(1) = INT64_MIN; \
if (decNumberIsSNaN(&dfp.b)) { \
invalid_flags |= FP_VXSNAN; \
} \
} \
dfp_set_FPSCR_flag(&dfp, invalid_flags, FP_VE); \
} else if (unlikely(decNumberIsZero(&dfp.b))) { \
- dfp.t64[0] = 0; \
+ dfp.vt.VsrD(1) = 0; \
} else { \
decNumberToIntegralExact(&dfp.b, &dfp.b, &dfp.context); \
- dfp.t64[0] = decNumberIntegralToInt64(&dfp.b, &dfp.context); \
+ dfp.vt.VsrD(1) = decNumberIntegralToInt64(&dfp.b, &dfp.context); \
if (decContextTestStatus(&dfp.context, DEC_Invalid_operation)) { \
- dfp.t64[0] = decNumberIsNegative(&dfp.b) ? INT64_MIN : INT64_MAX; \
+ dfp.vt.VsrD(1) = decNumberIsNegative(&dfp.b) ? INT64_MIN : \
+ INT64_MAX; \
dfp_set_FPSCR_flag(&dfp, FP_VX | FP_VXCVI, FP_VE); \
} else { \
dfp_check_for_XX(&dfp); \
} \
} \
\
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
}
DFP_HELPER_CTFIX(dctfix, 64)
@@ -1075,11 +1077,11 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
decNumberGetBCD(&dfp.b, digits); \
- dfp.t64[0] = dfp.t64[1] = 0; \
+ dfp.vt.VsrD(0) = dfp.vt.VsrD(1) = 0; \
N = dfp.b.digits; \
\
for (i = 0; (i < N) && (i < (size)/4); i++) { \
- dfp_set_bcd_digit_##size(dfp.t64, digits[N-i-1], i); \
+ dfp_set_bcd_digit_##size(&dfp.vt.u64[0], digits[N - i - 1], i); \
} \
\
if (sp & 2) { \
@@ -1090,13 +1092,13 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
} else { \
sgn = ((sp & 1) ? 0xF : 0xC); \
} \
- dfp_set_sign_##size(dfp.t64, sgn); \
+ dfp_set_sign_##size(&dfp.vt.u64[0], sgn); \
} \
\
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -1126,7 +1128,8 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
decNumberZero(&dfp.t); \
\
if (s) { \
- uint8_t sgnNibble = dfp_get_bcd_digit_##size(dfp.b64, offset++); \
+ uint8_t sgnNibble = dfp_get_bcd_digit_##size(&dfp.vb.u64[0], \
+ offset++); \
switch (sgnNibble) { \
case 0xD: \
case 0xB: \
@@ -1146,7 +1149,8 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
\
while (offset < (size) / 4) { \
n++; \
- digits[(size) / 4 - n] = dfp_get_bcd_digit_##size(dfp.b64, offset++); \
+ digits[(size) / 4 - n] = dfp_get_bcd_digit_##size(&dfp.vb.u64[0], \
+ offset++); \
if (digits[(size) / 4 - n] > 10) { \
dfp_set_FPSCR_flag(&dfp, FP_VX | FP_VXCVI, FPSCR_VE); \
return; \
@@ -1165,9 +1169,9 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
dfp_finalize_decimal##size(&dfp); \
dfp_set_FPRF_from_FRT(&dfp); \
if ((size) == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if ((size) == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -1178,30 +1182,30 @@ DFP_HELPER_ENBCD(denbcdq, 128)
void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
- uint64_t t64; \
+ ppc_vsr_t vt; \
\
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
if (unlikely(decNumberIsSpecial(&dfp.b))) { \
if (decNumberIsInfinite(&dfp.b)) { \
- t64 = -1; \
+ vt.VsrD(1) = -1; \
} else if (decNumberIsSNaN(&dfp.b)) { \
- t64 = -3; \
+ vt.VsrD(1) = -3; \
} else if (decNumberIsQNaN(&dfp.b)) { \
- t64 = -2; \
+ vt.VsrD(1) = -2; \
} else { \
assert(0); \
} \
- set_dfp64(t, &t64); \
+ set_dfp64(t, &vt); \
} else { \
if ((size) == 64) { \
- t64 = dfp.b.exponent + 398; \
+ vt.VsrD(1) = dfp.b.exponent + 398; \
} else if ((size) == 128) { \
- t64 = dfp.b.exponent + 6176; \
+ vt.VsrD(1) = dfp.b.exponent + 6176; \
} else { \
assert(0); \
} \
- set_dfp64(t, &t64); \
+ set_dfp64(t, &vt); \
} \
}
@@ -1225,12 +1229,13 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
ppc_fprp_t *b) \
{ \
struct PPC_DFP dfp; \
- uint64_t raw_qnan, raw_snan, raw_inf, max_exp, a64; \
+ uint64_t raw_qnan, raw_snan, raw_inf, max_exp; \
+ ppc_vsr_t va; \
int bias; \
int64_t exp; \
\
- get_dfp64(&a64, a); \
- exp = (int64_t)a64; \
+ get_dfp64(&va, a); \
+ exp = (int64_t)va.VsrD(1); \
dfp_prepare_decimal##size(&dfp, 0, b, env); \
\
if ((size) == 64) { \
@@ -1250,14 +1255,14 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
} \
\
if (unlikely((exp < 0) || (exp > max_exp))) { \
- dfp.t64[0] = dfp.b64[0]; \
- dfp.t64[1] = dfp.b64[1]; \
+ dfp.vt.VsrD(0) = dfp.vb.VsrD(0); \
+ dfp.vt.VsrD(1) = dfp.vb.VsrD(1); \
if (exp == -1) { \
- dfp_set_raw_exp_##size(dfp.t64, raw_inf); \
+ dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_inf); \
} else if (exp == -3) { \
- dfp_set_raw_exp_##size(dfp.t64, raw_snan); \
+ dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_snan); \
} else { \
- dfp_set_raw_exp_##size(dfp.t64, raw_qnan); \
+ dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_qnan); \
} \
} else { \
dfp.t = dfp.b; \
@@ -1268,9 +1273,9 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp_finalize_decimal##size(&dfp); \
} \
if (size == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else if (size == 128) { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
@@ -1348,20 +1353,21 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp_finalize_decimal##size(&dfp); \
} else { \
if ((size) == 64) { \
- dfp.t64[0] = dfp.a64[0] & 0xFFFC000000000000ULL; \
- dfp_clear_lmd_from_g5msb(dfp.t64); \
+ dfp.vt.VsrD(1) = dfp.va.VsrD(1) & \
+ 0xFFFC000000000000ULL; \
+ dfp_clear_lmd_from_g5msb(&dfp.vt.VsrD(1)); \
} else { \
- dfp.t64[HI_IDX] = dfp.a64[HI_IDX] & \
- 0xFFFFC00000000000ULL; \
- dfp_clear_lmd_from_g5msb(dfp.t64 + HI_IDX); \
- dfp.t64[LO_IDX] = 0; \
+ dfp.vt.VsrD(0) = dfp.va.VsrD(0) & \
+ 0xFFFFC00000000000ULL; \
+ dfp_clear_lmd_from_g5msb(&dfp.vt.VsrD(0)); \
+ dfp.vt.VsrD(1) = 0; \
} \
} \
\
if ((size) == 64) { \
- set_dfp64(t, dfp.t64); \
+ set_dfp64(t, &dfp.vt); \
} else { \
- set_dfp128(t, dfp.t64); \
+ set_dfp128(t, &dfp.vt); \
} \
}
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
` (4 preceding siblings ...)
2019-09-24 15:35 ` [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 21:44 ` Mark Cave-Ayland
2019-09-24 21:46 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 7/7] target/ppc: remove unnecessary if() around calls to set_dfp{64, 128}() in DFP macros Mark Cave-Ayland
2019-09-24 16:30 ` [PATCH 0/7] target/ppc: DFP fixes and improvements Paul Clarke
7 siblings, 2 replies; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
Switch over all accesses to the decimal numbers held in struct PPC_DFP from
using HI_IDX and LO_IDX to using the VsrD() macro instead. Not only does this
allow the compiler to ensure that the various dfp_* functions are being passed
a ppc_vsr_t rather than an arbitrary uint64_t pointer, but also allows the
host endian-specific HI_IDX and LO_IDX to be completely removed from
dfp_helper.c.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/dfp_helper.c | 70 ++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 39 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index ed437f97da..c2d335e928 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -28,13 +28,6 @@
#include "libdecnumber/dpd/decimal64.h"
#include "libdecnumber/dpd/decimal128.h"
-#if defined(HOST_WORDS_BIGENDIAN)
-#define HI_IDX 0
-#define LO_IDX 1
-#else
-#define HI_IDX 1
-#define LO_IDX 0
-#endif
static void get_dfp64(ppc_vsr_t *dst, ppc_fprp_t *dfp)
{
@@ -1039,31 +1032,31 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
DFP_HELPER_CTFIX(dctfix, 64)
DFP_HELPER_CTFIX(dctfixq, 128)
-static inline void dfp_set_bcd_digit_64(uint64_t *t, uint8_t digit,
- unsigned n)
+static inline void dfp_set_bcd_digit_64(ppc_vsr_t *t, uint8_t digit,
+ unsigned n)
{
- *t |= ((uint64_t)(digit & 0xF) << (n << 2));
+ t->VsrD(1) |= ((uint64_t)(digit & 0xF) << (n << 2));
}
-static inline void dfp_set_bcd_digit_128(uint64_t *t, uint8_t digit,
- unsigned n)
+static inline void dfp_set_bcd_digit_128(ppc_vsr_t *t, uint8_t digit,
+ unsigned n)
{
- t[(n & 0x10) ? HI_IDX : LO_IDX] |=
+ t->VsrD((n & 0x10) ? 0 : 1) |=
((uint64_t)(digit & 0xF) << ((n & 15) << 2));
}
-static inline void dfp_set_sign_64(uint64_t *t, uint8_t sgn)
+static inline void dfp_set_sign_64(ppc_vsr_t *t, uint8_t sgn)
{
- *t <<= 4;
- *t |= (sgn & 0xF);
+ t->VsrD(1) <<= 4;
+ t->VsrD(1) |= (sgn & 0xF);
}
-static inline void dfp_set_sign_128(uint64_t *t, uint8_t sgn)
+static inline void dfp_set_sign_128(ppc_vsr_t *t, uint8_t sgn)
{
- t[HI_IDX] <<= 4;
- t[HI_IDX] |= (t[LO_IDX] >> 60);
- t[LO_IDX] <<= 4;
- t[LO_IDX] |= (sgn & 0xF);
+ t->VsrD(0) <<= 4;
+ t->VsrD(0) |= (t->VsrD(0) >> 60);
+ t->VsrD(1) <<= 4;
+ t->VsrD(1) |= (sgn & 0xF);
}
#define DFP_HELPER_DEDPD(op, size) \
@@ -1081,7 +1074,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
N = dfp.b.digits; \
\
for (i = 0; (i < N) && (i < (size)/4); i++) { \
- dfp_set_bcd_digit_##size(&dfp.vt.u64[0], digits[N - i - 1], i); \
+ dfp_set_bcd_digit_##size(&dfp.vt, digits[N - i - 1], i); \
} \
\
if (sp & 2) { \
@@ -1092,7 +1085,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
} else { \
sgn = ((sp & 1) ? 0xF : 0xC); \
} \
- dfp_set_sign_##size(&dfp.vt.u64[0], sgn); \
+ dfp_set_sign_##size(&dfp.vt, sgn); \
} \
\
if (size == 64) { \
@@ -1105,14 +1098,14 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
DFP_HELPER_DEDPD(ddedpd, 64)
DFP_HELPER_DEDPD(ddedpdq, 128)
-static inline uint8_t dfp_get_bcd_digit_64(uint64_t *t, unsigned n)
+static inline uint8_t dfp_get_bcd_digit_64(ppc_vsr_t *t, unsigned n)
{
- return *t >> ((n << 2) & 63) & 15;
+ return t->VsrD(1) >> ((n << 2) & 63) & 15;
}
-static inline uint8_t dfp_get_bcd_digit_128(uint64_t *t, unsigned n)
+static inline uint8_t dfp_get_bcd_digit_128(ppc_vsr_t *t, unsigned n)
{
- return t[(n & 0x10) ? HI_IDX : LO_IDX] >> ((n << 2) & 63) & 15;
+ return t->VsrD((n & 0x10) ? 0 : 1) >> ((n << 2) & 63) & 15;
}
#define DFP_HELPER_ENBCD(op, size) \
@@ -1128,8 +1121,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
decNumberZero(&dfp.t); \
\
if (s) { \
- uint8_t sgnNibble = dfp_get_bcd_digit_##size(&dfp.vb.u64[0], \
- offset++); \
+ uint8_t sgnNibble = dfp_get_bcd_digit_##size(&dfp.vb, offset++); \
switch (sgnNibble) { \
case 0xD: \
case 0xB: \
@@ -1149,7 +1141,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
\
while (offset < (size) / 4) { \
n++; \
- digits[(size) / 4 - n] = dfp_get_bcd_digit_##size(&dfp.vb.u64[0], \
+ 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); \
@@ -1212,16 +1204,16 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
DFP_HELPER_XEX(dxex, 64)
DFP_HELPER_XEX(dxexq, 128)
-static void dfp_set_raw_exp_64(uint64_t *t, uint64_t raw)
+static void dfp_set_raw_exp_64(ppc_vsr_t *t, uint64_t raw)
{
- *t &= 0x8003ffffffffffffULL;
- *t |= (raw << (63 - 13));
+ t->VsrD(1) &= 0x8003ffffffffffffULL;
+ t->VsrD(1) |= (raw << (63 - 13));
}
-static void dfp_set_raw_exp_128(uint64_t *t, uint64_t raw)
+static void dfp_set_raw_exp_128(ppc_vsr_t *t, uint64_t raw)
{
- t[HI_IDX] &= 0x80003fffffffffffULL;
- t[HI_IDX] |= (raw << (63 - 17));
+ t->VsrD(0) &= 0x80003fffffffffffULL;
+ t->VsrD(0) |= (raw << (63 - 17));
}
#define DFP_HELPER_IEX(op, size) \
@@ -1258,11 +1250,11 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp.vt.VsrD(0) = dfp.vb.VsrD(0); \
dfp.vt.VsrD(1) = dfp.vb.VsrD(1); \
if (exp == -1) { \
- dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_inf); \
+ dfp_set_raw_exp_##size(&dfp.vt, raw_inf); \
} else if (exp == -3) { \
- dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_snan); \
+ dfp_set_raw_exp_##size(&dfp.vt, raw_snan); \
} else { \
- dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_qnan); \
+ dfp_set_raw_exp_##size(&dfp.vt, raw_qnan); \
} \
} else { \
dfp.t = dfp.b; \
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 7/7] target/ppc: remove unnecessary if() around calls to set_dfp{64, 128}() in DFP macros
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
` (5 preceding siblings ...)
2019-09-24 15:35 ` [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c Mark Cave-Ayland
@ 2019-09-24 15:35 ` Mark Cave-Ayland
2019-09-24 21:47 ` Richard Henderson
2019-09-24 16:30 ` [PATCH 0/7] target/ppc: DFP fixes and improvements Paul Clarke
7 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 15:35 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
Now that the parameters to both set_dfp64() and set_dfp128() are exactly the
same, there is no need for an explicit if() statement to determine which
function should be called based upon size. Instead we can simply use the
preprocessor to generate the call to set_dfp##size() directly.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/dfp_helper.c | 60 +++++++----------------------------------
1 file changed, 10 insertions(+), 50 deletions(-)
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index c2d335e928..8100bfc98d 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -427,11 +427,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dnop(&dfp.t, &dfp.a, &dfp.b, &dfp.context); \
dfp_finalize_decimal##size(&dfp); \
postprocs(&dfp); \
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
static void ADD_PPs(struct PPC_DFP *dfp)
@@ -747,11 +743,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_QUAI(dquai, 64)
@@ -769,11 +761,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_QUA(dqua, 64)
@@ -851,11 +839,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp_finalize_decimal##size(&dfp); \
QUA_PPs(&dfp); \
\
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_RRND(drrnd, 64)
@@ -874,11 +858,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
dfp_finalize_decimal##size(&dfp); \
postprocs(&dfp); \
\
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
static void RINTX_PPs(struct PPC_DFP *dfp)
@@ -978,11 +958,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
dfp_finalize_decimal##size(&dfp); \
CFFIX_PPs(&dfp); \
\
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
static void CFFIX_PPs(struct PPC_DFP *dfp)
@@ -1088,11 +1064,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
dfp_set_sign_##size(&dfp.vt, sgn); \
} \
\
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_DEDPD(ddedpd, 64)
@@ -1160,11 +1132,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
} \
dfp_finalize_decimal##size(&dfp); \
dfp_set_FPRF_from_FRT(&dfp); \
- if ((size) == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if ((size) == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_ENBCD(denbcd, 64)
@@ -1264,11 +1232,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
dfp.t.exponent = exp - bias; \
dfp_finalize_decimal##size(&dfp); \
} \
- if (size == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else if (size == 128) { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_IEX(diex, 64)
@@ -1356,11 +1320,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
} \
} \
\
- if ((size) == 64) { \
- set_dfp64(t, &dfp.vt); \
- } else { \
- set_dfp128(t, &dfp.vt); \
- } \
+ set_dfp##size(t, &dfp.vt); \
}
DFP_HELPER_SHIFT(dscli, 64, 1)
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 0/7] target/ppc: DFP fixes and improvements
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
` (6 preceding siblings ...)
2019-09-24 15:35 ` [PATCH 7/7] target/ppc: remove unnecessary if() around calls to set_dfp{64, 128}() in DFP macros Mark Cave-Ayland
@ 2019-09-24 16:30 ` Paul Clarke
2019-09-24 16:37 ` Mark Cave-Ayland
7 siblings, 1 reply; 23+ messages in thread
From: Paul Clarke @ 2019-09-24 16:30 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, david
Mark,
What tree / branch would these patches apply to? (I've tried qemu master, dgibson master and ppc-for-4.2, or I'm doing something wrong.)
PC
On 9/24/19 10:35 AM, Mark Cave-Ayland wrote:
> This patchset fixes the DFP issue reported at https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.launchpad.net_qemu_-2Bbug_1841990&d=DwIDAg&c=jf_iaSHvJObTbx-siA1ZOg&r=0emNUfh2Pr0wDtXKlYJSCQ&m=NtaEgUJhN3SbT5hKyyOdgnt5ArLSzDw2l22WvleDMmU&s=qIBW6IX8pu3ej_AJj-toJH8PmhrvgJaXDJgbg1tgbY8&e=
> caused by the change in FP register storage in commit ef96e3ae96 "target/ppc:
> move FP and VMX registers into aligned vsr register array" along with some
> further tidy-up/improvements.
>
> Patches 1 and 2 introduce get/set helper functions for reading and writing
> DFP even-odd register pairs (rather than accessing the register pointers
> directly) which then leads to the real fix in patch 3.
>
> Following on from this patches 4 to 6 change the struct PPC_DFP internal
> decimal representation from uint64[2] to ppc_vsr_t which enables us to use
> the existing VsrD() macro to access the correct elements regardless of host
> endian and remove the explicit HI_IDX and LO_IDX references.
>
> Finally patch 7 simplifies the calls to set_dfp{64,128}() in DFP macros
> which can now be generated directly by the preprocessor rather than requiring
> an explicit if() statement.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
>
> Mark Cave-Ayland (7):
> target/ppc: introduce get_dfp{64,128}() helper functions
> target/ppc: introduce set_dfp{64,128}() helper functions
> target/ppc: update {get,set}_dfp{64,128}() helper functions to
> read/write DFP numbers correctly
> target/ppc: introduce dfp_finalize_decimal{64,128}() helper functions
> target/ppc: change struct PPC_DFP decimal storage from uint64[2] to
> ppc_vsr_t
> target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX
> from dfp_helper.c
> target/ppc: remove unnecessary if() around calls to set_dfp{64,128}()
> in DFP macros
>
> target/ppc/cpu.h | 1 +
> target/ppc/dfp_helper.c | 384 ++++++++++++++++++++--------------------
> target/ppc/helper.h | 2 +-
> 3 files changed, 193 insertions(+), 194 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/7] target/ppc: DFP fixes and improvements
2019-09-24 16:30 ` [PATCH 0/7] target/ppc: DFP fixes and improvements Paul Clarke
@ 2019-09-24 16:37 ` Mark Cave-Ayland
0 siblings, 0 replies; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 16:37 UTC (permalink / raw)
To: Paul Clarke, qemu-devel, qemu-ppc, david
On 24/09/2019 17:30, Paul Clarke wrote:
> Mark,
>
> What tree / branch would these patches apply to? (I've tried qemu master, dgibson master and ppc-for-4.2, or I'm doing something wrong.)
>
> PC
Hi Paul,
I've just checked my local repo and I can confirm they are based on qemu master
commit 8dc57281b8 "Merge remote-tracking branch
'remotes/cleber/tags/python-next-pull-request' into staging".
ATB,
Mark.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions
2019-09-24 15:35 ` [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions Mark Cave-Ayland
@ 2019-09-24 19:21 ` Richard Henderson
2019-09-24 21:05 ` Mark Cave-Ayland
0 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 19:21 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> +static void get_dfp64(uint64_t *dst, uint64_t *dfp)
> +{
> + dst[0] = dfp[0];
> +}
> +
> +static void get_dfp128(uint64_t *dst, uint64_t *dfp)
> +{
> + dst[0] = dfp[HI_IDX];
> + dst[1] = dfp[LO_IDX];
> +}
I'm not keen on this. I would prefer some type difference that prevents you
from getting the arguments the wrong way around.
I'm thinking that a combination helper like
static void get_dfp64(decNumber *out, uint64_t *in)
{
union {
uint64_t i;
decimal64 d;
} u;
u.i = *in;
decimal64ToNumber(&u.d, out);
}
> @@ -129,7 +140,7 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, uint64_t *a,
> dfp->env = env;
>
> if (a) {
> - dfp->a64[0] = *a;
> + get_dfp64(dfp->a64, a);
> decimal64ToNumber((decimal64 *)dfp->a64, &dfp->a);
> } else {
> dfp->a64[0] = 0;
becomes
get_dfp64(&dfp->a, a);
and that might clean up a lot of the code.
> @@ -617,10 +626,12 @@ uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint64_t *b) \
> { \
> struct PPC_DFP dfp; \
> unsigned k; \
> + uint64_t a64; \
> \
> dfp_prepare_decimal##size(&dfp, 0, b, env); \
> \
> - k = *a & 0x3F; \
> + get_dfp64(&a64, a); \
> + k = a64 & 0x3F; \
> \
> if (unlikely(decNumberIsSpecial(&dfp.b))) { \
> dfp.crbf = 1; \
Whereas cases like this, where a scalar is being passed in, I don't think that
re-using the same helper is appropriate. Ideally, they would be passed in by
value, and not by reference at all. That, of course, requires changes to the
translator beyond the scope of this patch.
> void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
> {
> struct PPC_DFP dfp;
> + uint64_t b64;
> dfp_prepare_decimal128(&dfp, 0, 0, env);
> - decimal64ToNumber((decimal64 *)b, &dfp.t);
> + get_dfp64(&b64, b);
> + decimal64ToNumber((decimal64 *)&b64, &dfp.t);
This would become
get_dfp64(&dfp.t, b);
with no intermediate b64 temp.
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions
2019-09-24 19:21 ` Richard Henderson
@ 2019-09-24 21:05 ` Mark Cave-Ayland
2019-09-24 21:29 ` Richard Henderson
0 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 21:05 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-ppc, pc, david
On 24/09/2019 20:21, Richard Henderson wrote:
> On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
>> +static void get_dfp64(uint64_t *dst, uint64_t *dfp)
>> +{
>> + dst[0] = dfp[0];
>> +}
>> +
>> +static void get_dfp128(uint64_t *dst, uint64_t *dfp)
>> +{
>> + dst[0] = dfp[HI_IDX];
>> + dst[1] = dfp[LO_IDX];
>> +}
>
> I'm not keen on this. I would prefer some type difference that prevents you
> from getting the arguments the wrong way around.
>
> I'm thinking that a combination helper like
>
> static void get_dfp64(decNumber *out, uint64_t *in)
> {
> union {
> uint64_t i;
> decimal64 d;
> } u;
>
> u.i = *in;
> decimal64ToNumber(&u.d, out);
> }
>
>> @@ -129,7 +140,7 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, uint64_t *a,
>> dfp->env = env;
>>
>> if (a) {
>> - dfp->a64[0] = *a;
>> + get_dfp64(dfp->a64, a);
>> decimal64ToNumber((decimal64 *)dfp->a64, &dfp->a);
>> } else {
>> dfp->a64[0] = 0;
>
> becomes
>
> get_dfp64(&dfp->a, a);
>
> and that might clean up a lot of the code.
Right, and in fact I had a similar thought myself regarding type safety since one of
the issues with working with the existing code in dfp_helper.c is that everything
uses a uint64_t * which makes it really difficult to figure out why things are
crashing if you make a typo :/
Note that this patch simply introduces the helpers without making changes to the
existing logic which is why both arguments are still uint64_t *. The real work is
done in patch 3 where ppc_fptr_t type is introduced, and also see the switch from
uint64_t * to ppc_vsr_t in patch 5.
With the full patchset applied you'll see that get_dfp64() and friends are in exactly
the same form you show above, and if I swap the arguments then the compiler does
actually complain, although somewhat cryptically.
>> @@ -617,10 +626,12 @@ uint32_t helper_##op(CPUPPCState *env, uint64_t *a, uint64_t *b) \
>> { \
>> struct PPC_DFP dfp; \
>> unsigned k; \
>> + uint64_t a64; \
>> \
>> dfp_prepare_decimal##size(&dfp, 0, b, env); \
>> \
>> - k = *a & 0x3F; \
>> + get_dfp64(&a64, a); \
>> + k = a64 & 0x3F; \
>> \
>> if (unlikely(decNumberIsSpecial(&dfp.b))) { \
>> dfp.crbf = 1; \
>
> Whereas cases like this, where a scalar is being passed in, I don't think that
> re-using the same helper is appropriate. Ideally, they would be passed in by
> value, and not by reference at all. That, of course, requires changes to the
> translator beyond the scope of this patch.
Agreed. I was really keen to avoid any translator changes if possible since the PPC
translator code tends to be quite tricky which is why I went with the above approach.
>> void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b)
>> {
>> struct PPC_DFP dfp;
>> + uint64_t b64;
>> dfp_prepare_decimal128(&dfp, 0, 0, env);
>> - decimal64ToNumber((decimal64 *)b, &dfp.t);
>> + get_dfp64(&b64, b);
>> + decimal64ToNumber((decimal64 *)&b64, &dfp.t);
>
> This would become
>
> get_dfp64(&dfp.t, b);
>
> with no intermediate b64 temp.
Funnily enough that did cross my mind, but I wasn't 100% sure that this wasn't doing
something extra within libdecnumber. If you think it makes sense then I can easily
add it into a v2.
ATB,
Mark.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] target/ppc: introduce set_dfp{64,128}() helper functions
2019-09-24 15:35 ` [PATCH 2/7] target/ppc: introduce set_dfp{64,128}() " Mark Cave-Ayland
@ 2019-09-24 21:27 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:27 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> The existing functions (now incorrectly) assume that the MSB and LSB of DFP
> numbers are stored as consecutive 64-bit words in memory. Instead of accessing
> the DFP numbers directly, introduce set_dfp{64,128}() helper functions to ease
> the switch to the correct representation.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/dfp_helper.c | 90 ++++++++++++++++++++++-------------------
> 1 file changed, 48 insertions(+), 42 deletions(-)
>
> diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
> index 354a4aa877..cb98780d20 100644
> --- a/target/ppc/dfp_helper.c
> +++ b/target/ppc/dfp_helper.c
> @@ -47,6 +47,17 @@ static void get_dfp128(uint64_t *dst, uint64_t *dfp)
> dst[1] = dfp[LO_IDX];
> }
>
> +static void set_dfp64(uint64_t *dfp, uint64_t *src)
> +{
> + dfp[0] = src[0];
> +}
> +
> +static void set_dfp128(uint64_t *dfp, uint64_t *src)
> +{
> + dfp[0] = src[HI_IDX];
> + dfp[1] = src[LO_IDX];
> +}
...
> decimal##size##FromNumber((decimal##size *)dfp.t64, &dfp.t, &dfp.context);\
> postprocs(&dfp); \
> if (size == 64) { \
> - t[0] = dfp.t64[0]; \
> + set_dfp64(t, dfp.t64); \
> } else if (size == 128) { \
> - t[0] = dfp.t64[HI_IDX]; \
> - t[1] = dfp.t64[LO_IDX]; \
> + set_dfp128(t, dfp.t64); \
> }
This is perhaps a bit harder to see than the get case, because POSTPROCS is in
the way.
However, we can guess, because postprocs has not been passed GETPC(), that it
cannot longjmp out, and therefore not interfere with writing back of the result
to the register file. And, as it turns out from inspection, the set of
postprocs also does not modify dfp->t64; only looks at dfp->context.status.
Therefore, as a first step we can move postprocs down, then as a second step
combine the conversion from decNumber (dfp->t) to decimal128, and then into the
register file (t).
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions
2019-09-24 21:05 ` Mark Cave-Ayland
@ 2019-09-24 21:29 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:29 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 2:05 PM, Mark Cave-Ayland wrote:
> With the full patchset applied you'll see that get_dfp64() and friends are
> in exactly the same form you show above, and if I swap the arguments then
> the compiler does actually complain, although somewhat cryptically.
Oh, good. I'll finish reading the whole set before making too many more
comments ahead of your actual steps.
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/7] target/ppc: update {get, set}_dfp{64, 128}() helper functions to read/write DFP numbers correctly
2019-09-24 15:35 ` [PATCH 3/7] target/ppc: update {get, set}_dfp{64, 128}() helper functions to read/write DFP numbers correctly Mark Cave-Ayland
@ 2019-09-24 21:33 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:33 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> Since commit ef96e3ae96 "target/ppc: move FP and VMX registers into aligned vsr
> register array" FP registers are no longer stored consecutively in memory and so
> the current method of combining FP register pairs into DFP numbers is incorrect.
>
> Firstly update the definition of the dh_*_fprp defines in helper.h to reflect
> that FP registers are now stored as part of an array of ppc_vsr_t elements
> rather than plain uint64_t elements, and then introduce a new ppc_fprp_t type
> which conceptually represents a DFP even-odd register pair to be consumed by the
> DFP helper functions.
>
> Finally update the new DFP {get,set}_dfp{64,128}() helper functions to convert
> between DFP numbers and DFP even-odd register pairs correctly, making use of the
> existing VsrD() macro to access the correct elements regardless of host endian.
>
> Fixes: ef96e3ae96 "target/ppc: move FP and VMX registers into aligned vsr register array"
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/cpu.h | 1 +
> target/ppc/dfp_helper.c | 80 +++++++++++++++++++++--------------------
> target/ppc/helper.h | 2 +-
> 3 files changed, 44 insertions(+), 39 deletions(-)
Yay! I really was getting ahead of things.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t
2019-09-24 15:35 ` [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t Mark Cave-Ayland
@ 2019-09-24 21:41 ` Richard Henderson
2019-09-24 21:46 ` Richard Henderson
1 sibling, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:41 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> struct PPC_DFP {
> CPUPPCState *env;
> - uint64_t t64[2], a64[2], b64[2];
> + ppc_vsr_t vt, va, vb;
This I don't think is a good idea. It's not a vsr_t.
I think this step would be clearer with
union {
decimal64 d;
uint64_t i;
} u;
union {
decimal128 d;
uint64_t i[2];
} u;
in the separate dfp_prepare_decimal{64,128} functions. Which is basically what
we have before, only smooshing the a64 and b64 scratch-pads together, and using
a nice union instead of an ugly cast.
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c
2019-09-24 15:35 ` [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c Mark Cave-Ayland
@ 2019-09-24 21:44 ` Mark Cave-Ayland
2019-09-24 21:46 ` Richard Henderson
1 sibling, 0 replies; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-24 21:44 UTC (permalink / raw)
To: qemu-devel, qemu-ppc, pc, david
On 24/09/2019 16:35, Mark Cave-Ayland wrote:
> Switch over all accesses to the decimal numbers held in struct PPC_DFP from
> using HI_IDX and LO_IDX to using the VsrD() macro instead. Not only does this
> allow the compiler to ensure that the various dfp_* functions are being passed
> a ppc_vsr_t rather than an arbitrary uint64_t pointer, but also allows the
> host endian-specific HI_IDX and LO_IDX to be completely removed from
> dfp_helper.c.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/dfp_helper.c | 70 ++++++++++++++++++-----------------------
> 1 file changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
> index ed437f97da..c2d335e928 100644
> --- a/target/ppc/dfp_helper.c
> +++ b/target/ppc/dfp_helper.c
> @@ -28,13 +28,6 @@
> #include "libdecnumber/dpd/decimal64.h"
> #include "libdecnumber/dpd/decimal128.h"
>
> -#if defined(HOST_WORDS_BIGENDIAN)
> -#define HI_IDX 0
> -#define LO_IDX 1
> -#else
> -#define HI_IDX 1
> -#define LO_IDX 0
> -#endif
>
> static void get_dfp64(ppc_vsr_t *dst, ppc_fprp_t *dfp)
> {
> @@ -1039,31 +1032,31 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
> DFP_HELPER_CTFIX(dctfix, 64)
> DFP_HELPER_CTFIX(dctfixq, 128)
>
> -static inline void dfp_set_bcd_digit_64(uint64_t *t, uint8_t digit,
> - unsigned n)
> +static inline void dfp_set_bcd_digit_64(ppc_vsr_t *t, uint8_t digit,
> + unsigned n)
> {
> - *t |= ((uint64_t)(digit & 0xF) << (n << 2));
> + t->VsrD(1) |= ((uint64_t)(digit & 0xF) << (n << 2));
> }
>
> -static inline void dfp_set_bcd_digit_128(uint64_t *t, uint8_t digit,
> - unsigned n)
> +static inline void dfp_set_bcd_digit_128(ppc_vsr_t *t, uint8_t digit,
> + unsigned n)
> {
> - t[(n & 0x10) ? HI_IDX : LO_IDX] |=
> + t->VsrD((n & 0x10) ? 0 : 1) |=
> ((uint64_t)(digit & 0xF) << ((n & 15) << 2));
> }
>
> -static inline void dfp_set_sign_64(uint64_t *t, uint8_t sgn)
> +static inline void dfp_set_sign_64(ppc_vsr_t *t, uint8_t sgn)
> {
> - *t <<= 4;
> - *t |= (sgn & 0xF);
> + t->VsrD(1) <<= 4;
> + t->VsrD(1) |= (sgn & 0xF);
> }
>
> -static inline void dfp_set_sign_128(uint64_t *t, uint8_t sgn)
> +static inline void dfp_set_sign_128(ppc_vsr_t *t, uint8_t sgn)
> {
> - t[HI_IDX] <<= 4;
> - t[HI_IDX] |= (t[LO_IDX] >> 60);
> - t[LO_IDX] <<= 4;
> - t[LO_IDX] |= (sgn & 0xF);
> + t->VsrD(0) <<= 4;
> + t->VsrD(0) |= (t->VsrD(0) >> 60) ^^^^^^^^^^
I've just noticed that I've made a typo here: the line above should of course read:
t->VsrD(0) |= (t->VsrD(1) >> 60);
> + t->VsrD(1) <<= 4;
> + t->VsrD(1) |= (sgn & 0xF);
> }
>
> #define DFP_HELPER_DEDPD(op, size) \
> @@ -1081,7 +1074,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
> N = dfp.b.digits; \
> \
> for (i = 0; (i < N) && (i < (size)/4); i++) { \
> - dfp_set_bcd_digit_##size(&dfp.vt.u64[0], digits[N - i - 1], i); \
> + dfp_set_bcd_digit_##size(&dfp.vt, digits[N - i - 1], i); \
> } \
> \
> if (sp & 2) { \
> @@ -1092,7 +1085,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
> } else { \
> sgn = ((sp & 1) ? 0xF : 0xC); \
> } \
> - dfp_set_sign_##size(&dfp.vt.u64[0], sgn); \
> + dfp_set_sign_##size(&dfp.vt, sgn); \
> } \
> \
> if (size == 64) { \
> @@ -1105,14 +1098,14 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
> DFP_HELPER_DEDPD(ddedpd, 64)
> DFP_HELPER_DEDPD(ddedpdq, 128)
>
> -static inline uint8_t dfp_get_bcd_digit_64(uint64_t *t, unsigned n)
> +static inline uint8_t dfp_get_bcd_digit_64(ppc_vsr_t *t, unsigned n)
> {
> - return *t >> ((n << 2) & 63) & 15;
> + return t->VsrD(1) >> ((n << 2) & 63) & 15;
> }
>
> -static inline uint8_t dfp_get_bcd_digit_128(uint64_t *t, unsigned n)
> +static inline uint8_t dfp_get_bcd_digit_128(ppc_vsr_t *t, unsigned n)
> {
> - return t[(n & 0x10) ? HI_IDX : LO_IDX] >> ((n << 2) & 63) & 15;
> + return t->VsrD((n & 0x10) ? 0 : 1) >> ((n << 2) & 63) & 15;
> }
>
> #define DFP_HELPER_ENBCD(op, size) \
> @@ -1128,8 +1121,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
> decNumberZero(&dfp.t); \
> \
> if (s) { \
> - uint8_t sgnNibble = dfp_get_bcd_digit_##size(&dfp.vb.u64[0], \
> - offset++); \
> + uint8_t sgnNibble = dfp_get_bcd_digit_##size(&dfp.vb, offset++); \
> switch (sgnNibble) { \
> case 0xD: \
> case 0xB: \
> @@ -1149,7 +1141,7 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b, \
> \
> while (offset < (size) / 4) { \
> n++; \
> - digits[(size) / 4 - n] = dfp_get_bcd_digit_##size(&dfp.vb.u64[0], \
> + 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); \
> @@ -1212,16 +1204,16 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *b) \
> DFP_HELPER_XEX(dxex, 64)
> DFP_HELPER_XEX(dxexq, 128)
>
> -static void dfp_set_raw_exp_64(uint64_t *t, uint64_t raw)
> +static void dfp_set_raw_exp_64(ppc_vsr_t *t, uint64_t raw)
> {
> - *t &= 0x8003ffffffffffffULL;
> - *t |= (raw << (63 - 13));
> + t->VsrD(1) &= 0x8003ffffffffffffULL;
> + t->VsrD(1) |= (raw << (63 - 13));
> }
>
> -static void dfp_set_raw_exp_128(uint64_t *t, uint64_t raw)
> +static void dfp_set_raw_exp_128(ppc_vsr_t *t, uint64_t raw)
> {
> - t[HI_IDX] &= 0x80003fffffffffffULL;
> - t[HI_IDX] |= (raw << (63 - 17));
> + t->VsrD(0) &= 0x80003fffffffffffULL;
> + t->VsrD(0) |= (raw << (63 - 17));
> }
>
> #define DFP_HELPER_IEX(op, size) \
> @@ -1258,11 +1250,11 @@ void helper_##op(CPUPPCState *env, ppc_fprp_t *t, ppc_fprp_t *a, \
> dfp.vt.VsrD(0) = dfp.vb.VsrD(0); \
> dfp.vt.VsrD(1) = dfp.vb.VsrD(1); \
> if (exp == -1) { \
> - dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_inf); \
> + dfp_set_raw_exp_##size(&dfp.vt, raw_inf); \
> } else if (exp == -3) { \
> - dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_snan); \
> + dfp_set_raw_exp_##size(&dfp.vt, raw_snan); \
> } else { \
> - dfp_set_raw_exp_##size(&dfp.vt.u64[0], raw_qnan); \
> + dfp_set_raw_exp_##size(&dfp.vt, raw_qnan); \
> } \
> } else { \
> dfp.t = dfp.b; \
ATB,
Mark.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c
2019-09-24 15:35 ` [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c Mark Cave-Ayland
2019-09-24 21:44 ` Mark Cave-Ayland
@ 2019-09-24 21:46 ` Richard Henderson
2019-09-25 20:37 ` Mark Cave-Ayland
1 sibling, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:46 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> Switch over all accesses to the decimal numbers held in struct PPC_DFP from
> using HI_IDX and LO_IDX to using the VsrD() macro instead. Not only does this
> allow the compiler to ensure that the various dfp_* functions are being passed
> a ppc_vsr_t rather than an arbitrary uint64_t pointer, but also allows the
> host endian-specific HI_IDX and LO_IDX to be completely removed from
> dfp_helper.c.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/dfp_helper.c | 70 ++++++++++++++++++-----------------------
> 1 file changed, 31 insertions(+), 39 deletions(-)
Ho hum, vs patch 5 that was me not realizing how many different places really
want to manipulate a 128-bit value. Do go ahead and keep ppc_vsr_t for now.
It does look like we might be well served by using Int128 at some point, so
that these operations can expand to int128_t on appropriate hw so that the
compiler can DTRT with these.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t
2019-09-24 15:35 ` [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t Mark Cave-Ayland
2019-09-24 21:41 ` Richard Henderson
@ 2019-09-24 21:46 ` Richard Henderson
1 sibling, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:46 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> There are several places in dfp_helper.c that access the decimal number
> representations in struct PPC_DFP via HI_IDX and LO_IDX defines which are set
> at the top of dfp_helper.c according to the host endian.
>
> However we can instead switch to using ppc_vsr_t for decimal numbers and then
> make subsequent use of the existing VsrD() macros to access the correct
> element regardless of host endian. Note that 64-bit decimals are stored in the
> LSB of ppc_vsr_t (equivalent to VsrD(1)).
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/dfp_helper.c | 210 +++++++++++++++++++++-------------------
> 1 file changed, 108 insertions(+), 102 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] target/ppc: remove unnecessary if() around calls to set_dfp{64, 128}() in DFP macros
2019-09-24 15:35 ` [PATCH 7/7] target/ppc: remove unnecessary if() around calls to set_dfp{64, 128}() in DFP macros Mark Cave-Ayland
@ 2019-09-24 21:47 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:47 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> Now that the parameters to both set_dfp64() and set_dfp128() are exactly the
> same, there is no need for an explicit if() statement to determine which
> function should be called based upon size. Instead we can simply use the
> preprocessor to generate the call to set_dfp##size() directly.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/dfp_helper.c | 60 +++++++----------------------------------
> 1 file changed, 10 insertions(+), 50 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/7] target/ppc: introduce dfp_finalize_decimal{64, 128}() helper functions
2019-09-24 15:35 ` [PATCH 4/7] target/ppc: introduce dfp_finalize_decimal{64, 128}() helper functions Mark Cave-Ayland
@ 2019-09-24 21:47 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-24 21:47 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
> Most of the DFP helper functions call decimal{64,128}FromNumber() just before
> returning in order to convert the decNumber stored in dfp.t64 back to a
> Decimal{64,128} to write back to the FP registers.
>
> Introduce new dfp_finalize_decimal{64,128}() helper functions which both enable
> the parameter list to be reduced considerably, and also help minimise the
> changes required in the next patch.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/dfp_helper.c | 42 ++++++++++++++++++++++-------------------
> 1 file changed, 23 insertions(+), 19 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c
2019-09-24 21:46 ` Richard Henderson
@ 2019-09-25 20:37 ` Mark Cave-Ayland
2019-09-26 17:28 ` Richard Henderson
0 siblings, 1 reply; 23+ messages in thread
From: Mark Cave-Ayland @ 2019-09-25 20:37 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-ppc, pc, david
On 24/09/2019 22:46, Richard Henderson wrote:
> On 9/24/19 8:35 AM, Mark Cave-Ayland wrote:
>> Switch over all accesses to the decimal numbers held in struct PPC_DFP from
>> using HI_IDX and LO_IDX to using the VsrD() macro instead. Not only does this
>> allow the compiler to ensure that the various dfp_* functions are being passed
>> a ppc_vsr_t rather than an arbitrary uint64_t pointer, but also allows the
>> host endian-specific HI_IDX and LO_IDX to be completely removed from
>> dfp_helper.c.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> target/ppc/dfp_helper.c | 70 ++++++++++++++++++-----------------------
>> 1 file changed, 31 insertions(+), 39 deletions(-)
>
> Ho hum, vs patch 5 that was me not realizing how many different places really
> want to manipulate a 128-bit value. Do go ahead and keep ppc_vsr_t for now.
Yes, it's a little bit confusing in places as some operations are done on the
decNumber whilst others are done on the decimal representation. After trying a few
different approaches, using ppc_vsr_t seemed to be the easiest and most readable
solution here.
I see now that you've given R-b tags for patches 3-7, and having slept on it I'm
inclined to leave patches 1-2 as they are now, i.e. no code changes other than
introducing the get/set helpers to help keep the patchset as mechanical as possible.
Do you think that seems a reasonable approach?
> It does look like we might be well served by using Int128 at some point, so
> that these operations can expand to int128_t on appropriate hw so that the
> compiler can DTRT with these.
Certainly ppc_vsr_t already has __uint128_t and Int128 elements but the impression I
got from the #ifdef is that not all compilers would support it? Although having said
that, making such a change is not something that's really on my radar.
ATB,
Mark.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c
2019-09-25 20:37 ` Mark Cave-Ayland
@ 2019-09-26 17:28 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2019-09-26 17:28 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pc, david
On 9/25/19 1:37 PM, Mark Cave-Ayland wrote:
> I see now that you've given R-b tags for patches 3-7, and having slept on it I'm
> inclined to leave patches 1-2 as they are now, i.e. no code changes other than
> introducing the get/set helpers to help keep the patchset as mechanical as possible.
> Do you think that seems a reasonable approach?
Yes, I should have gone back and given you r-b for patches 1 & 2 as well.
Have those now:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Certainly ppc_vsr_t already has __uint128_t and Int128 elements but the impression I
> got from the #ifdef is that not all compilers would support it? Although having said
> that, making such a change is not something that's really on my radar.
Int128 is usable everywhere. It's just the implementation under the hood that
changes depending on the compiler.
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2019-09-26 17:32 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-24 15:35 [PATCH 0/7] target/ppc: DFP fixes and improvements Mark Cave-Ayland
2019-09-24 15:35 ` [PATCH 1/7] target/ppc: introduce get_dfp{64,128}() helper functions Mark Cave-Ayland
2019-09-24 19:21 ` Richard Henderson
2019-09-24 21:05 ` Mark Cave-Ayland
2019-09-24 21:29 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 2/7] target/ppc: introduce set_dfp{64,128}() " Mark Cave-Ayland
2019-09-24 21:27 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 3/7] target/ppc: update {get, set}_dfp{64, 128}() helper functions to read/write DFP numbers correctly Mark Cave-Ayland
2019-09-24 21:33 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 4/7] target/ppc: introduce dfp_finalize_decimal{64, 128}() helper functions Mark Cave-Ayland
2019-09-24 21:47 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 5/7] target/ppc: change struct PPC_DFP decimal storage from uint64[2] to ppc_vsr_t Mark Cave-Ayland
2019-09-24 21:41 ` Richard Henderson
2019-09-24 21:46 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 6/7] target/ppc: use existing VsrD() macro to eliminate HI_IDX and LO_IDX from dfp_helper.c Mark Cave-Ayland
2019-09-24 21:44 ` Mark Cave-Ayland
2019-09-24 21:46 ` Richard Henderson
2019-09-25 20:37 ` Mark Cave-Ayland
2019-09-26 17:28 ` Richard Henderson
2019-09-24 15:35 ` [PATCH 7/7] target/ppc: remove unnecessary if() around calls to set_dfp{64, 128}() in DFP macros Mark Cave-Ayland
2019-09-24 21:47 ` Richard Henderson
2019-09-24 16:30 ` [PATCH 0/7] target/ppc: DFP fixes and improvements Paul Clarke
2019-09-24 16:37 ` Mark Cave-Ayland
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.