Re: [PATCH 15/29] target/ppc: move various conditional branch insns to decodetree

[Miles Glenn <milesg@linux.ibm.com>]

Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn

On Thu, 2026-08-20 at 19:30 +0530, Chinmay Rath wrote:
> From: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> 
> Convert the following instructions to decodetree specification:
> 
> bc
> bca
> bcl
> bcla
> 
> bclr
> bclrl
> 
> bcctr
> bcctrl
> 
> bctar
> bctarl
> 
> The branch was tested by comparing the qemu -D log -d op,in_asm output for
> each instruction and also for the various combinations of branch option
> (BO) bits for the conditional branch instructions. Additionally, gdb
> was used to confirm the LR and other register's behavior is consistent
> to legacy behavior.
> 
> Further, the changes also pass a boot test into Fedora distro.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
> ---
>  target/ppc/insn32.decode               |  13 ++
>  target/ppc/translate.c                 | 148 -----------------------
>  target/ppc/translate/branch-impl.c.inc | 157 +++++++++++++++++++++++++
>  3 files changed, 170 insertions(+), 148 deletions(-)
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 6a3254aaf6..67fb181542 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -1495,6 +1495,19 @@ CLRBHRB         011111 ----- ----- ----- 0110101110 -
>  
>  B               010010 ........................ . .  @I_b
>  
> +%bd             2:14 !function=times_4
> +&bcond          bo:uint32_t bi bd aa:bool lk:bool
> +@B_bcond        ...... bo:5 bi:5 .............. aa:1 lk:1 &bcond bd=%bd
> +
> +BC              010000 ..... ..... .............. . .  @B_bcond
> +
> +&bclr          bo bi bh lk:bool
> +@XL_bclr       ...... bo:5 bi:5 --- bh:2 .......... lk:1 &bclr
> +
> +BCLR            010011 ..... ..... ---.. 0000010000 . @XL_bclr
> +BCCTR           010011 ..... ..... ---.. 1000010000 . @XL_bclr
> +BCTAR           010011 ..... ..... ---.. 1000110000 . @XL_bclr
> +
>  ## Misc POWER instructions
>  
>  ATTN            000000 00000 00000 00000 0100000000 0
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b528cab17d..7100787a25 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -3089,150 +3089,6 @@ static inline void gen_setlr(DisasContext *ctx, target_ulong nip)
>      tcg_gen_movi_tl(cpu_lr, nip);
>  }
>  
> -#define BCOND_IM  0
> -#define BCOND_LR  1
> -#define BCOND_CTR 2
> -#define BCOND_TAR 3
> -
> -static void gen_bcond(DisasContext *ctx, int type)
> -{
> -    uint32_t bo = BO(ctx->opcode);
> -    TCGLabel *l1;
> -    TCGv target;
> -    target_long bhrb_type = BHRB_TYPE_OTHER;
> -
> -    if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
> -        target = tcg_temp_new();
> -        if (type == BCOND_CTR) {
> -            tcg_gen_mov_tl(target, cpu_ctr);
> -        } else if (type == BCOND_TAR) {
> -            gen_load_spr(target, SPR_TAR);
> -        } else {
> -            tcg_gen_mov_tl(target, cpu_lr);
> -        }
> -        if (!LK(ctx->opcode)) {
> -            bhrb_type |= BHRB_TYPE_INDIRECT;
> -        }
> -        bhrb_type |= BHRB_TYPE_XL_FORM;
> -    } else {
> -        target = NULL;
> -    }
> -    if (LK(ctx->opcode)) {
> -        gen_setlr(ctx, ctx->base.pc_next);
> -        bhrb_type |= BHRB_TYPE_CALL;
> -    }
> -    l1 = gen_new_label();
> -    if ((bo & 0x4) == 0) {
> -        /* Decrement and test CTR */
> -        TCGv temp = tcg_temp_new();
> -
> -        if (type == BCOND_CTR) {
> -            /*
> -             * All ISAs up to v3 describe this form of bcctr as invalid but
> -             * some processors, ie. 64-bit server processors compliant with
> -             * arch 2.x, do implement a "test and decrement" logic instead,
> -             * as described in their respective UMs. This logic involves CTR
> -             * to act as both the branch target and a counter, which makes
> -             * it basically useless and thus never used in real code.
> -             *
> -             * This form was hence chosen to trigger extra micro-architectural
> -             * side-effect on real HW needed for the Spectre v2 workaround.
> -             * It is up to guests that implement such workaround, ie. linux, to
> -             * use this form in a way it just triggers the side-effect without
> -             * doing anything else harmful.
> -             */
> -            if (unlikely(!is_book3s_arch2x(ctx))) {
> -                gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> -                return;
> -            }
> -
> -            if (NARROW_MODE(ctx)) {
> -                tcg_gen_ext32u_tl(temp, cpu_ctr);
> -            } else {
> -                tcg_gen_mov_tl(temp, cpu_ctr);
> -            }
> -            if (bo & 0x2) {
> -                tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1);
> -            } else {
> -                tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1);
> -            }
> -            tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1);
> -        } else {
> -            tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1);
> -            if (NARROW_MODE(ctx)) {
> -                tcg_gen_ext32u_tl(temp, cpu_ctr);
> -            } else {
> -                tcg_gen_mov_tl(temp, cpu_ctr);
> -            }
> -            if (bo & 0x2) {
> -                tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1);
> -            } else {
> -                tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1);
> -            }
> -        }
> -        bhrb_type |= BHRB_TYPE_COND;
> -    }
> -    if ((bo & 0x10) == 0) {
> -        /* Test CR */
> -        uint32_t bi = BI(ctx->opcode);
> -        uint32_t mask = 0x08 >> (bi & 0x03);
> -        TCGv_i32 temp = tcg_temp_new_i32();
> -
> -        if (bo & 0x8) {
> -            tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask);
> -            tcg_gen_brcondi_i32(TCG_COND_EQ, temp, 0, l1);
> -        } else {
> -            tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask);
> -            tcg_gen_brcondi_i32(TCG_COND_NE, temp, 0, l1);
> -        }
> -        bhrb_type |= BHRB_TYPE_COND;
> -    }
> -
> -    gen_update_branch_history(ctx, ctx->cia, target, bhrb_type);
> -
> -    if (type == BCOND_IM) {
> -        target_ulong li = (target_long)((int16_t)(BD(ctx->opcode)));
> -        if (likely(AA(ctx->opcode) == 0)) {
> -            gen_goto_tb(ctx, 0, ctx->cia + li);
> -        } else {
> -            gen_goto_tb(ctx, 0, li);
> -        }
> -    } else {
> -        if (NARROW_MODE(ctx)) {
> -            tcg_gen_andi_tl(cpu_nip, target, (uint32_t)~3);
> -        } else {
> -            tcg_gen_andi_tl(cpu_nip, target, ~3);
> -        }
> -        gen_lookup_and_goto_ptr(ctx);
> -    }
> -    if ((bo & 0x14) != 0x14) {
> -        /* fallthrough case */
> -        gen_set_label(l1);
> -        gen_goto_tb(ctx, 1, ctx->base.pc_next);
> -    }
> -    ctx->base.is_jmp = DISAS_NORETURN;
> -}
> -
> -static void gen_bc(DisasContext *ctx)
> -{
> -    gen_bcond(ctx, BCOND_IM);
> -}
> -
> -static void gen_bcctr(DisasContext *ctx)
> -{
> -    gen_bcond(ctx, BCOND_CTR);
> -}
> -
> -static void gen_bclr(DisasContext *ctx)
> -{
> -    gen_bcond(ctx, BCOND_LR);
> -}
> -
> -static void gen_bctar(DisasContext *ctx)
> -{
> -    gen_bcond(ctx, BCOND_TAR);
> -}
> -
>  /***                      Condition register logical                       ***/
>  #define GEN_CRLOGIC(name, tcg_op, opc)                                        \
>  static void glue(gen_, name)(DisasContext *ctx)                               \
> @@ -5343,10 +5199,6 @@ GEN_HANDLER(stswx, 0x1F, 0x15, 0x14, 0x00000001, PPC_STRING),
>  /* ISA v3.0 changed the extended opcode from 62 to 30 */
>  GEN_HANDLER(wait, 0x1F, 0x1E, 0x01, 0x039FF801, PPC_WAIT),
>  GEN_HANDLER_E(wait, 0x1F, 0x1E, 0x00, 0x039CF801, PPC_NONE, PPC2_ISA300),
> -GEN_HANDLER(bc, 0x10, 0xFF, 0xFF, 0x00000000, PPC_FLOW),
> -GEN_HANDLER(bcctr, 0x13, 0x10, 0x10, 0x00000000, PPC_FLOW),
> -GEN_HANDLER(bclr, 0x13, 0x10, 0x00, 0x00000000, PPC_FLOW),
> -GEN_HANDLER_E(bctar, 0x13, 0x10, 0x11, 0x0000E000, PPC_NONE, PPC2_BCTAR_ISA207),
>  GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_FLOW),
>  #if defined(TARGET_PPC64)
>  GEN_HANDLER(rfid, 0x13, 0x12, 0x00, 0x03FF8001, PPC_64B),
> diff --git a/target/ppc/translate/branch-impl.c.inc b/target/ppc/translate/branch-impl.c.inc
> index 745f71afd1..c2889c1d12 100644
> --- a/target/ppc/translate/branch-impl.c.inc
> +++ b/target/ppc/translate/branch-impl.c.inc
> @@ -55,3 +55,160 @@ static bool trans_B(DisasContext *ctx, arg_I_b *a)
>  
>      return true;
>  }
> +
> +#define BCOND_IM  0
> +#define BCOND_LR  1
> +#define BCOND_CTR 2
> +#define BCOND_TAR 3
> +
> +static bool bcond_helper(DisasContext *ctx, int type, uint32_t bo, int bi,
> +                         int bd, int bh, bool aa, bool lk)
> +{
> +    TCGLabel *l1;
> +    TCGv target;
> +    target_long bhrb_type = BHRB_TYPE_OTHER;
> +
> +    if (type == BCOND_IM && bh != -1) {
> +        /* BCOND_IM should never use bh */
> +        return false;
> +    } else if (type != BCOND_IM && (bd != -1 || aa != 0)) {
> +        /* Other BCOND types should never use bd or aa */
> +        return false;
> +    }
> +
> +    if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
> +        target = tcg_temp_new();
> +        if (type == BCOND_CTR) {
> +            tcg_gen_mov_tl(target, cpu_ctr);
> +        } else if (type == BCOND_TAR) {
> +            gen_load_spr(target, SPR_TAR);
> +        } else {
> +            tcg_gen_mov_tl(target, cpu_lr);
> +        }
> +        if (!lk) {
> +            bhrb_type |= BHRB_TYPE_INDIRECT;
> +        }
> +        bhrb_type |= BHRB_TYPE_XL_FORM;
> +    } else {
> +        target = NULL;
> +    }
> +    if (lk) {
> +        gen_setlr(ctx, ctx->base.pc_next);
> +        bhrb_type |= BHRB_TYPE_CALL;
> +    }
> +    l1 = gen_new_label();
> +    if ((bo & 0x4) == 0) {
> +        /* Decrement and test CTR */
> +        TCGv temp = tcg_temp_new();
> +
> +        if (type == BCOND_CTR) {
> +            /*
> +             * All ISAs up to v3 describe this form of bcctr as invalid but
> +             * some processors, ie. 64-bit server processors compliant with
> +             * arch 2.x, do implement a "test and decrement" logic instead,
> +             * as described in their respective UMs. This logic involves CTR
> +             * to act as both the branch target and a counter, which makes
> +             * it basically useless and thus never used in real code.
> +             *
> +             * This form was hence chosen to trigger extra micro-architectural
> +             * side-effect on real HW needed for the Spectre v2 workaround.
> +             * It is up to guests that implement such workaround, ie. linux, to
> +             * use this form in a way it just triggers the side-effect without
> +             * doing anything else harmful.
> +             */
> +            if (unlikely(!is_book3s_arch2x(ctx))) {
> +                gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> +                return true;
> +            }
> +
> +            if (NARROW_MODE(ctx)) {
> +                tcg_gen_ext32u_tl(temp, cpu_ctr);
> +            } else {
> +                tcg_gen_mov_tl(temp, cpu_ctr);
> +            }
> +            if (bo & 0x2) {
> +                tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1);
> +            } else {
> +                tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1);
> +            }
> +            tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1);
> +        } else {
> +            tcg_gen_subi_tl(cpu_ctr, cpu_ctr, 1);
> +            if (NARROW_MODE(ctx)) {
> +                tcg_gen_ext32u_tl(temp, cpu_ctr);
> +            } else {
> +                tcg_gen_mov_tl(temp, cpu_ctr);
> +            }
> +            if (bo & 0x2) {
> +                tcg_gen_brcondi_tl(TCG_COND_NE, temp, 0, l1);
> +            } else {
> +                tcg_gen_brcondi_tl(TCG_COND_EQ, temp, 0, l1);
> +            }
> +        }
> +        bhrb_type |= BHRB_TYPE_COND;
> +    }
> +    if ((bo & 0x10) == 0) {
> +        /* Test CR */
> +        uint32_t mask = 0x08 >> (bi & 0x03);
> +        TCGv_i32 temp = tcg_temp_new_i32();
> +
> +        if (bo & 0x8) {
> +            tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask);
> +            tcg_gen_brcondi_i32(TCG_COND_EQ, temp, 0, l1);
> +        } else {
> +            tcg_gen_andi_i32(temp, cpu_crf[bi >> 2], mask);
> +            tcg_gen_brcondi_i32(TCG_COND_NE, temp, 0, l1);
> +        }
> +        bhrb_type |= BHRB_TYPE_COND;
> +    }
> +
> +    gen_update_branch_history(ctx, ctx->cia, target, bhrb_type);
> +
> +    if (type == BCOND_IM) {
> +        target_ulong li = (target_long)((int16_t)(bd));
> +        if (likely(aa == 0)) {
> +            gen_goto_tb(ctx, 0, ctx->cia + li);
> +        } else {
> +            gen_goto_tb(ctx, 0, li);
> +        }
> +    } else {
> +        if (NARROW_MODE(ctx)) {
> +            tcg_gen_andi_tl(cpu_nip, target, (uint32_t)~3);
> +        } else {
> +            tcg_gen_andi_tl(cpu_nip, target, ~3);
> +        }
> +        gen_lookup_and_goto_ptr(ctx);
> +    }
> +    if ((bo & 0x14) != 0x14) {
> +        /* fallthrough case */
> +        gen_set_label(l1);
> +        gen_goto_tb(ctx, 1, ctx->base.pc_next);
> +    }
> +    ctx->base.is_jmp = DISAS_NORETURN;
> +
> +    return true;
> +}
> +
> +static bool trans_BC(DisasContext *ctx, arg_bcond *a)
> +{
> +    /*
> +     * bh is not used for bc variants hence we pass -1
> +     */
> +    return bcond_helper(ctx, BCOND_IM, a->bo, a->bi, a->bd, -1, a->aa, a->lk);
> +}
> +
> +/*
> + * This helper is shared by bclr, bcctr and bctar.
> + */
> +static bool bclr_helper(DisasContext *ctx, arg_bclr *a, int type)
> +{
> +    /*
> +     * bd and aa is not used for bc variants hence we pass -1 and 0 respectively
> +     */
> +    return bcond_helper(ctx, type, a->bo, a->bi, -1, a->bh, 0, a->lk);
> +}
> +
> +TRANS(BCLR, bclr_helper, BCOND_LR)
> +TRANS(BCCTR, bclr_helper, BCOND_CTR)
> +TRANS_FLAGS2(BCTAR_ISA207, BCTAR, bclr_helper, BCOND_TAR)
> +