Re: [PATCH v7 03/10] target/ppc: Add gen_exception_err_nip() function

[Nicholas Piggin]

On Wed Feb 28, 2024 at 1:08 AM AEST, BALATON Zoltan wrote:
> Add gen_exception_err_nip() that does the same as gen_exception_err()
> but takes the nip as a parameter to allow specifying it instead of
> using the current instruction address then change gen_exception_err()
> to use it.
>
> The gen_exception() and gen_exception_nip() functions are similar so
> remove code duplication from those too while at it.
>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Thanks for doing this one

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  target/ppc/translate.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 28fc7791af..d87aae0cc4 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -296,33 +296,26 @@ static inline void gen_update_nip(DisasContext *ctx, 
> target_ulong nip)
>      tcg_gen_movi_tl(cpu_nip, nip);
>  }
>  
> -static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t 
> error)
> +static void gen_exception_err_nip(DisasContext *ctx, uint32_t excp,
> +                                  uint32_t error, target_ulong nip)
>  {
>      TCGv_i32 t0, t1;
>  
> -    /*
> -     * These are all synchronous exceptions, we set the PC back to the
> -     * faulting instruction
> -     */
> -    gen_update_nip(ctx, ctx->cia);
> +    gen_update_nip(ctx, nip);
>      t0 = tcg_constant_i32(excp);
>      t1 = tcg_constant_i32(error);
>      gen_helper_raise_exception_err(tcg_env, t0, t1);
>      ctx->base.is_jmp = DISAS_NORETURN;
>  }
>  
> -static void gen_exception(DisasContext *ctx, uint32_t excp)
> +static inline void gen_exception_err(DisasContext *ctx, uint32_t excp,
> +                                     uint32_t error)
>  {
> -    TCGv_i32 t0;
> -
>      /*
>       * These are all synchronous exceptions, we set the PC back to the
>       * faulting instruction
>       */
> -    gen_update_nip(ctx, ctx->cia);
> -    t0 = tcg_constant_i32(excp);
> -    gen_helper_raise_exception(tcg_env, t0);
> -    ctx->base.is_jmp = DISAS_NORETURN;
> +    gen_exception_err_nip(ctx, excp, error, ctx->cia);
>  }
>  
>  static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
> @@ -336,6 +329,15 @@ static void gen_exception_nip(DisasContext *ctx, 
> uint32_t excp,
>      ctx->base.is_jmp = DISAS_NORETURN;
>  }
>  
> +static inline void gen_exception(DisasContext *ctx, uint32_t excp)
> +{
> +    /*
> +     * These are all synchronous exceptions, we set the PC back to the
> +     * faulting instruction
> +     */
> +    gen_exception_nip(ctx, excp, ctx->cia);
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  static void gen_ppc_maybe_interrupt(DisasContext *ctx)
>  {