Since commit 8a14d31b00 "target/ppc: switch fpr/vsrl registers so all VSX
registers are in host endian order" functions getVSR() and putVSR() which used
to convert the VSR registers into host endian order are no longer required.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
target/ppc/mem_helper.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 5b0f9ee50d..17a3c931a9 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -417,26 +417,27 @@ STVE(stvewx, cpu_stl_data_ra, bswap32, u32)
void helper_##name(CPUPPCState *env, target_ulong addr, \
target_ulong xt_num, target_ulong rb) \
{ \
- int i; \
- ppc_vsr_t xt; \
+ ppc_vsr_t *xt = &env->vsr[xt_num]; \
+ ppc_vsr_t t; \
uint64_t nb = GET_NB(rb); \
+ int i; \
\
- xt.s128 = int128_zero(); \
+ t.s128 = int128_zero(); \
if (nb) { \
nb = (nb >= 16) ? 16 : nb; \
if (msr_le && !lj) { \
for (i = 16; i > 16 - nb; i--) { \
- xt.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
+ t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
addr = addr_add(env, addr, 1); \
} \
} else { \
for (i = 0; i < nb; i++) { \
- xt.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
+ t.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
addr = addr_add(env, addr, 1); \
} \
} \
} \
- putVSR(xt_num, &xt, env); \
+ *xt = t; \
}
VSX_LXVL(lxvl, 0)
@@ -447,26 +448,28 @@ VSX_LXVL(lxvll, 1)
void helper_##name(CPUPPCState *env, target_ulong addr, \
target_ulong xt_num, target_ulong rb) \
{ \
- int i; \
- ppc_vsr_t xt; \
+ ppc_vsr_t *xt = &env->vsr[xt_num]; \
+ ppc_vsr_t t = *xt; \
target_ulong nb = GET_NB(rb); \
+ int i; \
\
if (!nb) { \
return; \
} \
- getVSR(xt_num, &xt, env); \
+ \
nb = (nb >= 16) ? 16 : nb; \
if (msr_le && !lj) { \
for (i = 16; i > 16 - nb; i--) { \
- cpu_stb_data_ra(env, addr, xt.VsrB(i - 1), GETPC()); \
+ cpu_stb_data_ra(env, addr, t.VsrB(i - 1), GETPC()); \
addr = addr_add(env, addr, 1); \
} \
} else { \
for (i = 0; i < nb; i++) { \
- cpu_stb_data_ra(env, addr, xt.VsrB(i), GETPC()); \
+ cpu_stb_data_ra(env, addr, t.VsrB(i), GETPC()) ; \
addr = addr_add(env, addr, 1); \
} \
} \
+ *xt = t; \
}
VSX_STXVL(stxvl, 0)
--
2.11.0
On Sun, Jun 02, 2019 at 12:08:50PM +0100, Mark Cave-Ayland wrote:
> Since commit 8a14d31b00 "target/ppc: switch fpr/vsrl registers so all VSX
> registers are in host endian order" functions getVSR() and putVSR() which used
> to convert the VSR registers into host endian order are no longer required.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/ppc/mem_helper.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index 5b0f9ee50d..17a3c931a9 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -417,26 +417,27 @@ STVE(stvewx, cpu_stl_data_ra, bswap32, u32)
> void helper_##name(CPUPPCState *env, target_ulong addr, \
> target_ulong xt_num, target_ulong rb) \
> { \
> - int i; \
> - ppc_vsr_t xt; \
> + ppc_vsr_t *xt = &env->vsr[xt_num]; \
> + ppc_vsr_t t; \
> uint64_t nb = GET_NB(rb); \
> + int i; \
> \
> - xt.s128 = int128_zero(); \
> + t.s128 = int128_zero(); \
> if (nb) { \
> nb = (nb >= 16) ? 16 : nb; \
> if (msr_le && !lj) { \
> for (i = 16; i > 16 - nb; i--) { \
> - xt.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
> + t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
> addr = addr_add(env, addr, 1); \
> } \
> } else { \
> for (i = 0; i < nb; i++) { \
> - xt.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
> + t.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
> addr = addr_add(env, addr, 1); \
> } \
> } \
> } \
> - putVSR(xt_num, &xt, env); \
> + *xt = t; \
> }
>
> VSX_LXVL(lxvl, 0)
> @@ -447,26 +448,28 @@ VSX_LXVL(lxvll, 1)
> void helper_##name(CPUPPCState *env, target_ulong addr, \
> target_ulong xt_num, target_ulong rb) \
> { \
> - int i; \
> - ppc_vsr_t xt; \
> + ppc_vsr_t *xt = &env->vsr[xt_num]; \
> + ppc_vsr_t t = *xt; \
> target_ulong nb = GET_NB(rb); \
> + int i; \
> \
> if (!nb) { \
> return; \
> } \
> - getVSR(xt_num, &xt, env); \
> + \
> nb = (nb >= 16) ? 16 : nb; \
> if (msr_le && !lj) { \
> for (i = 16; i > 16 - nb; i--) { \
> - cpu_stb_data_ra(env, addr, xt.VsrB(i - 1), GETPC()); \
> + cpu_stb_data_ra(env, addr, t.VsrB(i - 1), GETPC()); \
> addr = addr_add(env, addr, 1); \
> } \
> } else { \
> for (i = 0; i < nb; i++) { \
> - cpu_stb_data_ra(env, addr, xt.VsrB(i), GETPC()); \
> + cpu_stb_data_ra(env, addr, t.VsrB(i), GETPC()) ; \
> addr = addr_add(env, addr, 1); \
> } \
> } \
> + *xt = t; \
Is this correct? AFAICT the original wasn't writing back, so why does
the new version?
> }
>
> VSX_STXVL(stxvl, 0)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 12/06/2019 02:04, David Gibson wrote:
> On Sun, Jun 02, 2019 at 12:08:50PM +0100, Mark Cave-Ayland wrote:
>> Since commit 8a14d31b00 "target/ppc: switch fpr/vsrl registers so all VSX
>> registers are in host endian order" functions getVSR() and putVSR() which used
>> to convert the VSR registers into host endian order are no longer required.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> target/ppc/mem_helper.c | 25 ++++++++++++++-----------
>> 1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
>> index 5b0f9ee50d..17a3c931a9 100644
>> --- a/target/ppc/mem_helper.c
>> +++ b/target/ppc/mem_helper.c
>> @@ -417,26 +417,27 @@ STVE(stvewx, cpu_stl_data_ra, bswap32, u32)
>> void helper_##name(CPUPPCState *env, target_ulong addr, \
>> target_ulong xt_num, target_ulong rb) \
>> { \
>> - int i; \
>> - ppc_vsr_t xt; \
>> + ppc_vsr_t *xt = &env->vsr[xt_num]; \
>> + ppc_vsr_t t; \
>> uint64_t nb = GET_NB(rb); \
>> + int i; \
>> \
>> - xt.s128 = int128_zero(); \
>> + t.s128 = int128_zero(); \
>> if (nb) { \
>> nb = (nb >= 16) ? 16 : nb; \
>> if (msr_le && !lj) { \
>> for (i = 16; i > 16 - nb; i--) { \
>> - xt.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
>> + t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } else { \
>> for (i = 0; i < nb; i++) { \
>> - xt.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
>> + t.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } \
>> } \
>> - putVSR(xt_num, &xt, env); \
>> + *xt = t; \
>> }
>>
>> VSX_LXVL(lxvl, 0)
>> @@ -447,26 +448,28 @@ VSX_LXVL(lxvll, 1)
>> void helper_##name(CPUPPCState *env, target_ulong addr, \
>> target_ulong xt_num, target_ulong rb) \
>> { \
>> - int i; \
>> - ppc_vsr_t xt; \
>> + ppc_vsr_t *xt = &env->vsr[xt_num]; \
>> + ppc_vsr_t t = *xt; \
>> target_ulong nb = GET_NB(rb); \
>> + int i; \
>> \
>> if (!nb) { \
>> return; \
>> } \
>> - getVSR(xt_num, &xt, env); \
>> + \
>> nb = (nb >= 16) ? 16 : nb; \
>> if (msr_le && !lj) { \
>> for (i = 16; i > 16 - nb; i--) { \
>> - cpu_stb_data_ra(env, addr, xt.VsrB(i - 1), GETPC()); \
>> + cpu_stb_data_ra(env, addr, t.VsrB(i - 1), GETPC()); \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } else { \
>> for (i = 0; i < nb; i++) { \
>> - cpu_stb_data_ra(env, addr, xt.VsrB(i), GETPC()); \
>> + cpu_stb_data_ra(env, addr, t.VsrB(i), GETPC()) ; \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } \
>> + *xt = t; \
>
> Is this correct? AFAICT the original wasn't writing back, so why does
> the new version?
Ooops yes, this shouldn't be here at all. I'll fix it up in the next version of the
series, along with Richard's comments about using the xt pointer directly.
>> }
>>
>> VSX_STXVL(stxvl, 0)
ATB,
Mark.
On 6/2/19 4:08 AM, Mark Cave-Ayland wrote:
> - getVSR(xt_num, &xt, env); \
> + \
> nb = (nb >= 16) ? 16 : nb; \
> if (msr_le && !lj) { \
> for (i = 16; i > 16 - nb; i--) { \
> - cpu_stb_data_ra(env, addr, xt.VsrB(i - 1), GETPC()); \
> + cpu_stb_data_ra(env, addr, t.VsrB(i - 1), GETPC()); \
> addr = addr_add(env, addr, 1); \
> } \
> } else { \
> for (i = 0; i < nb; i++) { \
> - cpu_stb_data_ra(env, addr, xt.VsrB(i), GETPC()); \
> + cpu_stb_data_ra(env, addr, t.VsrB(i), GETPC()) ; \
> addr = addr_add(env, addr, 1); \
> } \
> } \
> + *xt = t; \
Do not write back stores.
Actually, in this case there's no reason to copy t = *xt. Just store directly
from xt->VsrB(i).
r~
On 12/06/2019 20:47, Richard Henderson wrote:
> On 6/2/19 4:08 AM, Mark Cave-Ayland wrote:
>> - getVSR(xt_num, &xt, env); \
>> + \
>> nb = (nb >= 16) ? 16 : nb; \
>> if (msr_le && !lj) { \
>> for (i = 16; i > 16 - nb; i--) { \
>> - cpu_stb_data_ra(env, addr, xt.VsrB(i - 1), GETPC()); \
>> + cpu_stb_data_ra(env, addr, t.VsrB(i - 1), GETPC()); \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } else { \
>> for (i = 0; i < nb; i++) { \
>> - cpu_stb_data_ra(env, addr, xt.VsrB(i), GETPC()); \
>> + cpu_stb_data_ra(env, addr, t.VsrB(i), GETPC()) ; \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } \
>> + *xt = t; \
>
> Do not write back stores.
Yeah, my mistake - David also managed to spot this one.
> Actually, in this case there's no reason to copy t = *xt. Just store directly
> from xt->VsrB(i).
Okay I'll fix that in v3.
ATB,
Mark.
© 2016 - 2026 Red Hat, Inc.