Re: [Qemu-devel] [PATCH 2/2] ppc: Convert op_andi to TCG

[Aurelien Jarno]

On Sun, Sep 14, 2008 at 01:23:42PM +0200, Andreas Färber wrote:
> Replace op_andi_... with tcg_gen_andi_tl.
>
> Signed-off-by: Andreas Faerber <address@hidden>

Applied, thanks.

> ---
>  This patch reveals a problem with the crf conversion to i32:
>  As you can see we now have a mix of _tl and _i32 for cpu_T[].
>  I would suggest to convert cpu_crf to tl to avoid problems
>  (it was converted from i8 to i32 previously).

While I agree there is a problem, I disagree about the solution. cpu_T[]
registers will eventually disappear. Changing things to match them is a
bad idea. The solution is to get rid of them in functions manipulating
cpu_crf. I'll commit a patch later to fix that.

>  target-ppc/op.c        |   27 ---------------------------
>  target-ppc/translate.c |   38 +++++++++++---------------------------
>  2 files changed, 11 insertions(+), 54 deletions(-)
>
> diff --git a/target-ppc/op.c b/target-ppc/op.c
> index 95ab8b9..6416659 100644
> --- a/target-ppc/op.c
> +++ b/target-ppc/op.c
> @@ -1153,33 +1153,6 @@ void OPPROTO op_andc (void)
>      RETURN();
>  }
>
> -/* andi. */
> -void OPPROTO op_andi_T0 (void)
> -{
> -    T0 &= (uint32_t)PARAM1;
> -    RETURN();
> -}
> -
> -void OPPROTO op_andi_T1 (void)
> -{
> -    T1 &= (uint32_t)PARAM1;
> -    RETURN();
> -}
> -
> -#if defined(TARGET_PPC64)
> -void OPPROTO op_andi_T0_64 (void)
> -{
> -    T0 &= ((uint64_t)PARAM1 << 32) | (uint64_t)PARAM2;
> -    RETURN();
> -}
> -
> -void OPPROTO op_andi_T1_64 (void)
> -{
> -    T1 &= ((uint64_t)PARAM1 << 32) | (uint64_t)PARAM2;
> -    RETURN();
> -}
> -#endif
> -
>  /* count leading zero */
>  void OPPROTO op_cntlzw (void)
>  {
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index cc1f836..56bdef2 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1235,7 +1235,7 @@ GEN_LOGICAL2(andc, 0x01, PPC_INTEGER);
>  GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER)
>  {
>      tcg_gen_mov_tl(cpu_T[0], cpu_gpr[rS(ctx->opcode)]);
> -    gen_op_andi_T0(UIMM(ctx->opcode));
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], UIMM(ctx->opcode));
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
>      gen_set_Rc0(ctx);
>  }
> @@ -1243,7 +1243,7 @@ GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF,  
> 0x00000000, PPC_INTEGER)
>  GEN_HANDLER2(andis_, "andis.", 0x1D, 0xFF, 0xFF, 0x00000000,  
> PPC_INTEGER)
>  {
>      tcg_gen_mov_tl(cpu_T[0], cpu_gpr[rS(ctx->opcode)]);
> -    gen_op_andi_T0(UIMM(ctx->opcode) << 16);
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], UIMM(ctx->opcode) << 16);
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
>      gen_set_Rc0(ctx);
>  }
> @@ -1458,8 +1458,8 @@ GEN_HANDLER(rlwimi, 0x14, 0xFF, 0xFF, 0x00000000, 
> PPC_INTEGER)
>      me += 32;
>  #endif
>      mask = MASK(mb, me);
> -    gen_op_andi_T0(mask);
> -    gen_op_andi_T1(~mask);
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], mask);
> +    tcg_gen_andi_tl(cpu_T[1], cpu_T[1], ~mask);
>      gen_op_or();
>   do_store:
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> @@ -1498,7 +1498,7 @@ GEN_HANDLER(rlwinm, 0x15, 0xFF, 0xFF, 0x00000000, 
> PPC_INTEGER)
>      mb += 32;
>      me += 32;
>  #endif
> -    gen_op_andi_T0(MASK(mb, me));
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
>   do_store:
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
>      if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1519,7 +1519,7 @@ GEN_HANDLER(rlwnm, 0x17, 0xFF, 0xFF, 0x00000000,  
> PPC_INTEGER)
>          mb += 32;
>          me += 32;
>  #endif
> -        gen_op_andi_T0(MASK(mb, me));
> +        tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
>      }
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
>      if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1558,22 +1558,6 @@ GEN_HANDLER2(name##3, stringify(name), opc1, opc2 
> | 0x11, 0xFF, 0x00000000,   \
>      gen_##name(ctx, 1, 1);                                               
>      \
>  }
>
> -static always_inline void gen_andi_T0_64 (DisasContext *ctx, uint64_t  
> mask)
> -{
> -    if (mask >> 32)
> -        gen_op_andi_T0_64(mask >> 32, mask & 0xFFFFFFFF);
> -    else
> -        gen_op_andi_T0(mask);
> -}
> -
> -static always_inline void gen_andi_T1_64 (DisasContext *ctx, uint64_t  
> mask)
> -{
> -    if (mask >> 32)
> -        gen_op_andi_T1_64(mask >> 32, mask & 0xFFFFFFFF);
> -    else
> -        gen_op_andi_T1(mask);
> -}
> -
>  static always_inline void gen_rldinm (DisasContext *ctx, uint32_t mb,
>                                        uint32_t me, uint32_t sh)
>  {
> @@ -1597,7 +1581,7 @@ static always_inline void gen_rldinm (DisasContext 
> *ctx, uint32_t mb,
>      }
>      gen_op_rotli64_T0(sh);
>   do_mask:
> -    gen_andi_T0_64(ctx, MASK(mb, me));
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
>   do_store:
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
>      if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1641,7 +1625,7 @@ static always_inline void gen_rldnm (DisasContext 
> *ctx, uint32_t mb,
>      tcg_gen_mov_tl(cpu_T[1], cpu_gpr[rB(ctx->opcode)]);
>      gen_op_rotl64_T0_T1();
>      if (unlikely(mb != 0 || me != 63)) {
> -        gen_andi_T0_64(ctx, MASK(mb, me));
> +        tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
>      }
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
>      if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1689,8 +1673,8 @@ static always_inline void gen_rldimi (DisasContext 
> *ctx, int mbn, int shn)
>      gen_op_rotli64_T0(sh);
>   do_mask:
>      mask = MASK(mb, me);
> -    gen_andi_T0_64(ctx, mask);
> -    gen_andi_T1_64(ctx, ~mask);
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], mask);
> +    tcg_gen_andi_tl(cpu_T[1], cpu_T[1], ~mask);
>      gen_op_or();
>   do_store:
>      tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> @@ -3107,7 +3091,7 @@ GEN_HANDLER(cr##op, 0x13, 0x01, opc, 0x00000001,  
> PPC_INTEGER)                 \
>          gen_op_sli_T1(-sh);                                              
>      \
>      gen_op_##op();                                                       
>      \
>      bitmask = 1 << (3 - (crbD(ctx->opcode) & 0x03));                     
>      \
> -    gen_op_andi_T0(bitmask);                                             
>      \
> +    tcg_gen_andi_tl(cpu_T[0], cpu_T[0], bitmask);                        
>      \
>      tcg_gen_andi_i32(cpu_T[1], cpu_crf[crbD(ctx->opcode) >> 2],  
> ~bitmask);    \
>      gen_op_or();                                                         
>      \
>      tcg_gen_andi_i32(cpu_crf[crbD(ctx->opcode) >> 2], cpu_T[0], 0xf);    
>      \
> -- 
> 1.5.5.1
>



-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net