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 | 24 +++++++++++-------------
target/ppc/translate/vsx-impl.inc.c | 8 +++++---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 5b0f9ee50d..4dfa7ee23f 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -415,28 +415,27 @@ STVE(stvewx, cpu_stl_data_ra, bswap32, u32)
#define VSX_LXVL(name, lj) \
void helper_##name(CPUPPCState *env, target_ulong addr, \
- target_ulong xt_num, target_ulong rb) \
+ target_ulong xt, target_ulong rb) \
{ \
+ ppc_vsr_t *r = &env->vsr[xt]; \
+ int nb = GET_NB(env->gpr[rb]); \
int i; \
- ppc_vsr_t xt; \
- uint64_t nb = GET_NB(rb); \
\
- xt.s128 = int128_zero(); \
+ r->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()); \
+ r->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()); \
+ r->VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
addr = addr_add(env, addr, 1); \
} \
} \
} \
- putVSR(xt_num, &xt, env); \
}
VSX_LXVL(lxvl, 0)
@@ -445,25 +444,24 @@ VSX_LXVL(lxvll, 1)
#define VSX_STXVL(name, lj) \
void helper_##name(CPUPPCState *env, target_ulong addr, \
- target_ulong xt_num, target_ulong rb) \
+ target_ulong xt, target_ulong rb) \
{ \
+ ppc_vsr_t *r = &env->vsr[xt]; \
+ int nb = GET_NB(env->gpr[rb]); \
int i; \
- ppc_vsr_t xt; \
- target_ulong nb = GET_NB(rb); \
\
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, r->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, r->VsrB(i), GETPC()); \
addr = addr_add(env, addr, 1); \
} \
} \
diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c
index 11d9b75d01..c776d50ddc 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -290,7 +290,7 @@ VSX_VECTOR_LOAD_STORE(stxvx, st_i64, 1)
#define VSX_VECTOR_LOAD_STORE_LENGTH(name) \
static void gen_##name(DisasContext *ctx) \
{ \
- TCGv EA, xt; \
+ TCGv EA, xt, rb; \
\
if (xT(ctx->opcode) < 32) { \
if (unlikely(!ctx->vsx_enabled)) { \
@@ -304,12 +304,14 @@ static void gen_##name(DisasContext *ctx) \
} \
} \
EA = tcg_temp_new(); \
- xt = tcg_const_tl(xT(ctx->opcode)); \
gen_set_access_type(ctx, ACCESS_INT); \
gen_addr_register(ctx, EA); \
- gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
+ xt = tcg_const_tl(xT(ctx->opcode)); \
+ rb = tcg_const_tl(rB(ctx->opcode)); \
+ gen_helper_##name(cpu_env, EA, xt, rb); \
tcg_temp_free(EA); \
tcg_temp_free(xt); \
+ tcg_temp_free(rb); \
}
VSX_VECTOR_LOAD_STORE_LENGTH(lxvl)
--
2.11.0
On 4/28/19 7:38 AM, Mark Cave-Ayland wrote:
> #define VSX_LXVL(name, lj) \
> void helper_##name(CPUPPCState *env, target_ulong addr, \
> - target_ulong xt_num, target_ulong rb) \
> + target_ulong xt, target_ulong rb) \
> { \
> + ppc_vsr_t *r = &env->vsr[xt]; \
> + int nb = GET_NB(env->gpr[rb]); \
> int i; \
> - ppc_vsr_t xt; \
> - uint64_t nb = GET_NB(rb); \
> \
> - xt.s128 = int128_zero(); \
> + r->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()); \
> + r->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()); \
> + r->VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
> addr = addr_add(env, addr, 1); \
> } \
> } \
> } \
> - putVSR(xt_num, &xt, env); \
> }
Similarly, this modifies env->vsr[xt] before all exceptions are recognized.
> @@ -304,12 +304,14 @@ static void gen_##name(DisasContext *ctx) \
> } \
> } \
> EA = tcg_temp_new(); \
> - xt = tcg_const_tl(xT(ctx->opcode)); \
> gen_set_access_type(ctx, ACCESS_INT); \
> gen_addr_register(ctx, EA); \
> - gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
> + xt = tcg_const_tl(xT(ctx->opcode)); \
> + rb = tcg_const_tl(rB(ctx->opcode)); \
> + gen_helper_##name(cpu_env, EA, xt, rb); \
> tcg_temp_free(EA); \
> tcg_temp_free(xt); \
> + tcg_temp_free(rb); \
> }
Why are you adjusting the function to pass the rB register number rather than
the contents of rB? That seems the wrong way around...
r~
On 30/04/2019 17:29, Richard Henderson wrote:
> On 4/28/19 7:38 AM, Mark Cave-Ayland wrote:
>> #define VSX_LXVL(name, lj) \
>> void helper_##name(CPUPPCState *env, target_ulong addr, \
>> - target_ulong xt_num, target_ulong rb) \
>> + target_ulong xt, target_ulong rb) \
>> { \
>> + ppc_vsr_t *r = &env->vsr[xt]; \
>> + int nb = GET_NB(env->gpr[rb]); \
>> int i; \
>> - ppc_vsr_t xt; \
>> - uint64_t nb = GET_NB(rb); \
>> \
>> - xt.s128 = int128_zero(); \
>> + r->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()); \
>> + r->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()); \
>> + r->VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC()); \
>> addr = addr_add(env, addr, 1); \
>> } \
>> } \
>> } \
>> - putVSR(xt_num, &xt, env); \
>> }
>
> Similarly, this modifies env->vsr[xt] before all exceptions are recognized.
Okay - if you're happy with my previous suggestion with the VSR* macros and the local
variable then I can make the same change here too.
>> @@ -304,12 +304,14 @@ static void gen_##name(DisasContext *ctx) \
>> } \
>> } \
>> EA = tcg_temp_new(); \
>> - xt = tcg_const_tl(xT(ctx->opcode)); \
>> gen_set_access_type(ctx, ACCESS_INT); \
>> gen_addr_register(ctx, EA); \
>> - gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
>> + xt = tcg_const_tl(xT(ctx->opcode)); \
>> + rb = tcg_const_tl(rB(ctx->opcode)); \
>> + gen_helper_##name(cpu_env, EA, xt, rb); \
>> tcg_temp_free(EA); \
>> tcg_temp_free(xt); \
>> + tcg_temp_free(rb); \
>> }
>
> Why are you adjusting the function to pass the rB register number rather than
> the contents of rB? That seems the wrong way around...
I think what I was trying to do here was eliminate the cpu_gpr since it feels to me
that with the vector patchsets and your negative offset patches that this should be
the way to go for accessing CPUState rather than using TCG globals.
Looking at this again I realise the solution is really the same as is currently used
for gen_load_spr() so I can use something like this:
static inline void gen_load_gpr(TCGv t, int reg)
{
tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, gpr[reg]));
}
Does this seem reasonable as a solution?
ATB,
Mark.
On 5/5/19 2:34 AM, Mark Cave-Ayland wrote:
>>> EA = tcg_temp_new(); \
>>> - xt = tcg_const_tl(xT(ctx->opcode)); \
>>> gen_set_access_type(ctx, ACCESS_INT); \
>>> gen_addr_register(ctx, EA); \
>>> - gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
>>> + xt = tcg_const_tl(xT(ctx->opcode)); \
>>> + rb = tcg_const_tl(rB(ctx->opcode)); \
>>> + gen_helper_##name(cpu_env, EA, xt, rb); \
>>> tcg_temp_free(EA); \
>>> tcg_temp_free(xt); \
>>> + tcg_temp_free(rb); \
>>> }
>>
>> Why are you adjusting the function to pass the rB register number rather than
>> the contents of rB? That seems the wrong way around...
>
> I think what I was trying to do here was eliminate the cpu_gpr since it
> feels to me that with the vector patchsets and your negative offset patches
> that this should be the way to go for accessing CPUState rather than using
> TCG globals.
Not for the integer register set.
> Looking at this again I realise the solution is really the same as is
> currently used
> for gen_load_spr() so I can use something like this:>
> static inline void gen_load_gpr(TCGv t, int reg)
> {
> tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, gpr[reg]));
> }
>
> Does this seem reasonable as a solution?
No, this will fail quickly.
r~
On 05/05/2019 15:34, Richard Henderson wrote:
> On 5/5/19 2:34 AM, Mark Cave-Ayland wrote:
>>>> EA = tcg_temp_new(); \
>>>> - xt = tcg_const_tl(xT(ctx->opcode)); \
>>>> gen_set_access_type(ctx, ACCESS_INT); \
>>>> gen_addr_register(ctx, EA); \
>>>> - gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
>>>> + xt = tcg_const_tl(xT(ctx->opcode)); \
>>>> + rb = tcg_const_tl(rB(ctx->opcode)); \
>>>> + gen_helper_##name(cpu_env, EA, xt, rb); \
>>>> tcg_temp_free(EA); \
>>>> tcg_temp_free(xt); \
>>>> + tcg_temp_free(rb); \
>>>> }
>>>
>>> Why are you adjusting the function to pass the rB register number rather than
>>> the contents of rB? That seems the wrong way around...
>>
>> I think what I was trying to do here was eliminate the cpu_gpr since it
>> feels to me that with the vector patchsets and your negative offset patches
>> that this should be the way to go for accessing CPUState rather than using
>> TCG globals.
>
> Not for the integer register set.
>
>> Looking at this again I realise the solution is really the same as is
>> currently used
>> for gen_load_spr() so I can use something like this:>
>> static inline void gen_load_gpr(TCGv t, int reg)
>> {
>> tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, gpr[reg]));
>> }
>>
>> Does this seem reasonable as a solution?
>
> No, this will fail quickly.
Okay in that case I'll leave it as-is. So just to satisfy my curiosity here: is the
problem here the mixing and matching of offsets and TCG globals, rather than the use
of offsets as done for the VMX/VSX registers?
ATB,
Mark.
On 5/5/19 8:49 AM, Mark Cave-Ayland wrote: > Okay in that case I'll leave it as-is. So just to satisfy my curiosity here: is the > problem here the mixing and matching of offsets and TCG globals, rather than the use > of offsets as done for the VMX/VSX registers? Correct. r~
© 2016 - 2026 Red Hat, Inc.