Re: [PATCH v4 2/9] target/arm: Change gen_goto_tb to work on displacements

[Philippe Mathieu-Daudé]

Hi Richard,

On 6/9/22 12:05, Richard Henderson wrote:
> In preparation for TARGET_TB_PCREL, reduce reliance on absolute values.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/translate-a64.c | 40 ++++++++++++++++++++------------------
>   target/arm/translate.c     | 10 ++++++----
>   2 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index b7787e7786..f7a13bddea 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -378,8 +378,10 @@ static inline bool use_goto_tb(DisasContext *s, uint64_t 
> dest)
>       return translator_use_goto_tb(&s->base, dest);
>   }
>   
> -static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
> +static void gen_goto_tb(DisasContext *s, int n, int diff)
>   {
> +    uint64_t dest = s->pc_curr + diff;
> +
>       if (use_goto_tb(s, dest)) {
>           tcg_gen_goto_tb(n);
>           gen_a64_set_pc_im(dest);
> @@ -1362,7 +1364,7 @@ static inline AArch64DecodeFn *lookup_disas_fn(const 
> AArch64DecodeTable *table,
>    */
>   static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
>   {
> -    uint64_t addr = s->pc_curr + sextract32(insn, 0, 26) * 4;
> +    int diff = sextract32(insn, 0, 26) * 4;
>   
>       if (insn & (1U << 31)) {
>           /* BL Branch with link */
> @@ -1371,7 +1373,7 @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t 
> insn)
>   
>       /* B Branch / BL Branch with link */
>       reset_btype(s);
> -    gen_goto_tb(s, 0, addr);
> +    gen_goto_tb(s, 0, diff);
>   }
>   
>   /* Compare and branch (immediate)
> @@ -1383,14 +1385,14 @@ static void disas_uncond_b_imm(DisasContext *s, 
> uint32_t insn)
>   static void disas_comp_b_imm(DisasContext *s, uint32_t insn)
>   {
>       unsigned int sf, op, rt;
> -    uint64_t addr;
> +    int diff;
>       TCGLabel *label_match;
>       TCGv_i64 tcg_cmp;
>   
>       sf = extract32(insn, 31, 1);
>       op = extract32(insn, 24, 1); /* 0: CBZ; 1: CBNZ */
>       rt = extract32(insn, 0, 5);
> -    addr = s->pc_curr + sextract32(insn, 5, 19) * 4;
> +    diff = sextract32(insn, 5, 19) * 4;
>   
>       tcg_cmp = read_cpu_reg(s, rt, sf);
>       label_match = gen_new_label();
> @@ -1399,9 +1401,9 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t 
> insn)
>       tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ,
>                           tcg_cmp, 0, label_match);
>   
> -    gen_goto_tb(s, 0, s->base.pc_next);
> +    gen_goto_tb(s, 0, 4);

Why not use curr_insn_len() here?

>       gen_set_label(label_match);
> -    gen_goto_tb(s, 1, addr);
> +    gen_goto_tb(s, 1, diff);
>   }