| Message ID | 1407785009-6538-2-git-send-email-tommusta@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 08/11/2014 09:23 AM, Tom Musta wrote: > The rlwinm specification includes the ROTL32 operation, which is defined > to be a left rotation of two copies of the least significant 32 bits of > the source GPR. > > The current implementation is incorrect on 64-bit implementations in that > it rotates a single copy of the least significant 32 bits, padding with > zeroes in the most significant bits. Yes, it does describe rotate_32 as a double-copy of the low 32 bits. But it also describes the mask as having "0 bits elsewhere". Thus, post mask, I don't see how you could distinguish the implementations. Have you an example that doesn't work with the current code? r~
On 08/15/2014 08:34 AM, Richard Henderson wrote: > On 08/11/2014 09:23 AM, Tom Musta wrote: >> The rlwinm specification includes the ROTL32 operation, which is defined >> to be a left rotation of two copies of the least significant 32 bits of >> the source GPR. >> >> The current implementation is incorrect on 64-bit implementations in that >> it rotates a single copy of the least significant 32 bits, padding with >> zeroes in the most significant bits. > > Yes, it does describe rotate_32 as a double-copy of the low 32 bits. But it > also describes the mask as having "0 bits elsewhere". Thus, post mask, I don't > see how you could distinguish the implementations. > > Have you an example that doesn't work with the current code? Let me guess the answer myself -- it's MB > ME, and wraparound of the mask. I think I was distracted by the text "For all the uses given above, the high-order 32 bits of register RA are cleared." without clearly reading the examples. r~
diff --git a/target-ppc/translate.c b/target-ppc/translate.c index b23933f..a27d063 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -1672,11 +1672,9 @@ static void gen_rlwinm(DisasContext *ctx) } else { TCGv t0 = tcg_temp_new(); #if defined(TARGET_PPC64) - TCGv_i32 t1 = tcg_temp_new_i32(); - tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]); - tcg_gen_rotli_i32(t1, t1, sh); - tcg_gen_extu_i32_i64(t0, t1); - tcg_temp_free_i32(t1); + tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)], + cpu_gpr[rS(ctx->opcode)], 32, 32); + tcg_gen_rotli_i64(t0, t0, sh); #else tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh); #endif
The rlwinm specification includes the ROTL32 operation, which is defined to be a left rotation of two copies of the least significant 32 bits of the source GPR. The current implementation is incorrect on 64-bit implementations in that it rotates a single copy of the least significant 32 bits, padding with zeroes in the most significant bits. Fix the code to properly implement this ROTL32 operation. Signed-off-by: Tom Musta <tommusta@gmail.com> --- target-ppc/translate.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)