Re: [PATCH v2 25/37] target/ppc: Move rlwimi, rlwinm instructions to decodetree

[Amit Machhiwal <amachhiw@linux.ibm.com>]

On 2026/08/26 10:38 AM, Chinmay Rath wrote:
> From: Tanushree Shah <tshah@linux.ibm.com>
> 
> -Moving the following instructions to decodetree specification:
>             rlwimi                           : M-form
>             rlwimi.                          : M-form
>             rlwinm                           : M-form
>             rlwinm.                          : M-form
>  The changes were verified by validating that the tcg ops generated by
>  those instructions remain the same, which were captured with the
>  "-d in_asm,op" flag.
> 
>  Additionally, validated using small assembly tests confirming the
>  destination register is correctly updated based on rotated and masked
>  source values and confirmed that the value was same before and after
>  the change
> 
> Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
> Reviewed-by: Glenn Miles <milesg@linux.ibm.com>
> Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
> ---
>  target/ppc/insn32.decode                   |   6 ++
>  target/ppc/translate.c                     | 106 ---------------------
>  target/ppc/translate/fixedpoint-impl.c.inc |  98 +++++++++++++++++++
>  3 files changed, 104 insertions(+), 106 deletions(-)
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index eed963bd71..8f027e7ff5 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -312,6 +312,9 @@
>  
>  @Z23_te_tbp     ...... ....0 te:5 ....0 rmc:2 ........ rc:1     &Z23_te_tb frt=%z23_frtp frb=%z23_frbp
>  
> +&M              ra rs sh mb me rc:bool
> +@M              ...... rs:5 ra:5 sh:5 mb:5 me:5 rc:1 &M
> +
>  ### Fixed-Point Load Instructions
>  
>  LBZ             100010 ..... ..... ................     @D
> @@ -561,6 +564,9 @@ SRADI           011111 ..... ..... ..... 110011101 . .  @XS
>  
>  EXTSWSLI        011111 ..... ..... ..... 110111101 . .  @XS
>  
> +RLWIMI          010100 ..... ..... ..... ..... ......   @M
> +RLWINM          010101 ..... ..... ..... ..... ......   @M
> +
>  ## BCD Assist
>  
>  ADDG6S          011111 ..... ..... ..... - 001001010 -  @X
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 712990cf2d..d2c51b4d59 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -2012,110 +2012,6 @@ static void gen_pause(DisasContext *ctx)
>  
>  /***                             Integer rotate                            ***/
>  
> -/* rlwimi & rlwimi. */
> -static void gen_rlwimi(DisasContext *ctx)
> -{
> -    TCGv t_ra = cpu_gpr[rA(ctx->opcode)];
> -    TCGv t_rs = cpu_gpr[rS(ctx->opcode)];
> -    uint32_t sh = SH(ctx->opcode);
> -    uint32_t mb = MB(ctx->opcode);
> -    uint32_t me = ME(ctx->opcode);
> -
> -    if (sh == (31 - me) && mb <= me) {
> -        tcg_gen_deposit_tl(t_ra, t_ra, t_rs, sh, me - mb + 1);
> -    } else {
> -        target_ulong mask;
> -        bool mask_in_32b = true;
> -        TCGv t1;
> -
> -#if defined(TARGET_PPC64)
> -        mb += 32;
> -        me += 32;
> -#endif
> -        mask = MASK(mb, me);
> -
> -#if defined(TARGET_PPC64)
> -        if (mask > 0xffffffffu) {
> -            mask_in_32b = false;
> -        }
> -#endif
> -        t1 = tcg_temp_new();
> -        if (mask_in_32b) {
> -            TCGv_i32 t0 = tcg_temp_new_i32();
> -            tcg_gen_trunc_tl_i32(t0, t_rs);
> -            tcg_gen_rotli_i32(t0, t0, sh);
> -            tcg_gen_extu_i32_tl(t1, t0);
> -        } else {
> -#if defined(TARGET_PPC64)
> -            tcg_gen_deposit_i64(t1, t_rs, t_rs, 32, 32);
> -            tcg_gen_rotli_i64(t1, t1, sh);
> -#else
> -            g_assert_not_reached();
> -#endif
> -        }
> -
> -        tcg_gen_andi_tl(t1, t1, mask);
> -        tcg_gen_andi_tl(t_ra, t_ra, ~mask);
> -        tcg_gen_or_tl(t_ra, t_ra, t1);
> -    }
> -    if (unlikely(Rc(ctx->opcode) != 0)) {
> -        gen_set_Rc0(ctx, t_ra);
> -    }
> -}
> -
> -/* rlwinm & rlwinm. */
> -static void gen_rlwinm(DisasContext *ctx)
> -{
> -    TCGv t_ra = cpu_gpr[rA(ctx->opcode)];
> -    TCGv t_rs = cpu_gpr[rS(ctx->opcode)];
> -    int sh = SH(ctx->opcode);
> -    int mb = MB(ctx->opcode);
> -    int me = ME(ctx->opcode);
> -    int len = me - mb + 1;
> -    int rsh = (32 - sh) & 31;
> -
> -    if (sh != 0 && len > 0 && me == (31 - sh)) {
> -        tcg_gen_deposit_z_tl(t_ra, t_rs, sh, len);
> -    } else if (me == 31 && rsh + len <= 32) {
> -        tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
> -    } else {
> -        target_ulong mask;
> -        bool mask_in_32b = true;
> -#if defined(TARGET_PPC64)
> -        mb += 32;
> -        me += 32;
> -#endif
> -        mask = MASK(mb, me);
> -#if defined(TARGET_PPC64)
> -        if (mask > 0xffffffffu) {
> -            mask_in_32b = false;
> -        }
> -#endif
> -        if (mask_in_32b) {
> -            if (sh == 0) {
> -                tcg_gen_andi_tl(t_ra, t_rs, mask);
> -            } else {
> -                TCGv_i32 t0 = tcg_temp_new_i32();
> -                tcg_gen_trunc_tl_i32(t0, t_rs);
> -                tcg_gen_rotli_i32(t0, t0, sh);
> -                tcg_gen_andi_i32(t0, t0, mask);
> -                tcg_gen_extu_i32_tl(t_ra, t0);
> -            }
> -        } else {
> -#if defined(TARGET_PPC64)
> -            tcg_gen_deposit_i64(t_ra, t_rs, t_rs, 32, 32);
> -            tcg_gen_rotli_i64(t_ra, t_ra, sh);
> -            tcg_gen_andi_i64(t_ra, t_ra, mask);
> -#else
> -            g_assert_not_reached();
> -#endif
> -        }
> -    }
> -    if (unlikely(Rc(ctx->opcode) != 0)) {
> -        gen_set_Rc0(ctx, t_ra);
> -    }
> -}
> -
>  /* rlwnm & rlwnm. */
>  static void gen_rlwnm(DisasContext *ctx)
>  {
> @@ -4807,8 +4703,6 @@ GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0xFFFFFFFF, PPC_NONE),
>  GEN_HANDLER_E(copy, 0x1F, 0x06, 0x18, 0x03C00001, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER_E(cp_abort, 0x1F, 0x06, 0x1A, 0x03FFF801, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER_E(paste, 0x1F, 0x06, 0x1C, 0x03C00000, PPC_NONE, PPC2_ISA300),
> -GEN_HANDLER(rlwimi, 0x14, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
> -GEN_HANDLER(rlwinm, 0x15, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
>  GEN_HANDLER(rlwnm, 0x17, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
>  /* handles lfdp, lxsd, lxssp */
>  GEN_HANDLER_E(dform39, 0x39, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA205),
> diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
> index 82fb645279..8fcee7a89d 100644
> --- a/target/ppc/translate/fixedpoint-impl.c.inc
> +++ b/target/ppc/translate/fixedpoint-impl.c.inc
> @@ -1874,6 +1874,104 @@ static bool trans_SRAWI(DisasContext *ctx, arg_SRAWI *a)
>      return true;
>  }
>  
> +static bool trans_RLWIMI(DisasContext *ctx, arg_RLWIMI *a)
> +{
> +    TCGv t_ra = cpu_gpr[a->ra];
> +    TCGv t_rs = cpu_gpr[a->rs];
> +
> +    if (a->sh == (31 - a->me) && a->mb <= a->me) {
> +        tcg_gen_deposit_tl(t_ra, t_ra, t_rs, a->sh, a->me - a->mb + 1);
> +    } else {
> +        target_ulong mask;
> +        bool mask_in_32b = true;
> +        TCGv t1;
> +
> +#if defined(TARGET_PPC64)
> +        a->mb += 32;
> +        a->me += 32;

The old gen_rlwimi/gen_rlwinm used local int mb/int me for this
adjustment, keeping the opcode struct immutable. Writing back into a->
fields is contrary to normal decodetree style — the arg_* struct is an
input, not a scratch pad. Please use local copies instead:

> +#endif
> +        mask = MASK((uint32_t)a->mb, (uint32_t)a->me);

MASK() takes uint64_t on PPC64 builds. Both translators in this patch
should use the same form. If the local-variable suggestion above is
adopted, the cast becomes unnecessary in both (the local int will
promote cleanly), and both can just use MASK(mb, me).

> +
> +#if defined(TARGET_PPC64)
> +        if (mask > 0xffffffffu) {
> +            mask_in_32b = false;
> +        }
> +#endif
> +        t1 = tcg_temp_new();
> +        if (mask_in_32b) {
> +            TCGv_i32 t0 = tcg_temp_new_i32();
> +            tcg_gen_trunc_tl_i32(t0, t_rs);
> +            tcg_gen_rotli_i32(t0, t0, a->sh);
> +            tcg_gen_extu_i32_tl(t1, t0);
> +        } else {
> +#if defined(TARGET_PPC64)
> +            tcg_gen_deposit_i64(t1, t_rs, t_rs, 32, 32);
> +            tcg_gen_rotli_i64(t1, t1, a->sh);
> +#else
> +            g_assert_not_reached();
> +#endif
> +        }
> +
> +        tcg_gen_andi_tl(t1, t1, mask);
> +        tcg_gen_andi_tl(t_ra, t_ra, ~mask);
> +        tcg_gen_or_tl(t_ra, t_ra, t1);
> +    }
> +    if (unlikely(a->rc)) {
> +        gen_set_Rc0(ctx, t_ra);
> +    }
> +    return true;
> +}
> +
> +static bool trans_RLWINM(DisasContext *ctx, arg_RLWINM *a)
> +{
> +    TCGv t_ra = cpu_gpr[a->ra];
> +    TCGv t_rs = cpu_gpr[a->rs];
> +    int len = a->me - a->mb + 1;
> +    int rsh = (32 - a->sh) & 31;
> +
> +    if (a->sh != 0 && len > 0 && a->me == (31 - a->sh)) {
> +        tcg_gen_deposit_z_tl(t_ra, t_rs, a->sh, len);
> +    } else if (a->me == 31 && rsh + len <= 32) {
> +        tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
> +    } else {
> +        target_ulong mask;
> +        bool mask_in_32b = true;
> +#if defined(TARGET_PPC64)
> +        a->mb += 32;
> +        a->me += 32;
> +#endif
> +        mask = MASK(a->mb, a->me);
> +#if defined(TARGET_PPC64)
> +        if (mask > 0xffffffffu) {
> +            mask_in_32b = false;
> +        }
> +#endif
> +        if (mask_in_32b) {
> +            if (a->sh == 0) {
> +                tcg_gen_andi_tl(t_ra, t_rs, mask);
> +            } else {
> +                TCGv_i32 t0 = tcg_temp_new_i32();
> +                tcg_gen_trunc_tl_i32(t0, t_rs);
> +                tcg_gen_rotli_i32(t0, t0, a->sh);
> +                tcg_gen_andi_i32(t0, t0, mask);
> +                tcg_gen_extu_i32_tl(t_ra, t0);
> +            }
> +        } else {
> +#if defined(TARGET_PPC64)
> +            tcg_gen_deposit_i64(t_ra, t_rs, t_rs, 32, 32);
> +            tcg_gen_rotli_i64(t_ra, t_ra, a->sh);
> +            tcg_gen_andi_i64(t_ra, t_ra, mask);
> +#else
> +            g_assert_not_reached();
> +#endif
> +        }
> +    }
> +    if (unlikely(a->rc)) {
> +        gen_set_Rc0(ctx, t_ra);
> +    }
> +    return true;
> +}
> +
>  static void do_fetch_inc_conditional(DisasContext *ctx, MemOp memop,
>                                       TCGv EA, int rt,
>                                       TCGCond cond, int addend)
> -- 
> 2.55.0
>