Re: [PATCH 03/15] Hexagon (target/hexagon) properly generate TB end for DISAS_NORETURN

[Richard Henderson]

On 3/24/21 8:49 PM, Taylor Simpson wrote:
> When exiting a TB, generate all the code before returning from
> hexagon_tr_translate_packet so that nothing needs to be done in
> hexagon_tr_tb_stop.
>
> Address feedback from Richard Henderson <richard.henderson@linaro.org>
>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
>   target/hexagon/translate.c | 62 +++++++++++++++++++++++++---------------------
>   target/hexagon/translate.h |  3 ---
>   2 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
> index 5d92ab0..19b9bc7 100644
> --- a/target/hexagon/translate.c
> +++ b/target/hexagon/translate.c
> @@ -54,16 +54,41 @@ static const char * const hexagon_prednames[] = {
>     "p0", "p1", "p2", "p3"
>   };
>   
> -void gen_exception(int excp)
> +static void gen_exception(int excp)

I would call this something like gen_exception_raw or gen_exception_nopc, or 
something because,

> +static void gen_exception_end_tb(DisasContext *ctx, int excp)

... *all* exceptions end the tb.

> +{
> +    gen_exec_counters(ctx);
> +    tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], hex_next_PC);
> +    gen_exception(excp);

The helper_raise_exception call longjmped away, so

> +    tcg_gen_exit_tb(NULL, 0);

... this exit_tb is dead code.

> @@ -537,8 +551,7 @@ static bool hexagon_tr_breakpoint_check(DisasContextBase 
> *dcbase, CPUState *cpu,
>       DisasContext *ctx = container_of(dcbase, DisasContext, base);
>   
>       tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], ctx->base.pc_next);
> -    ctx->base.is_jmp = DISAS_NORETURN;
> -    gen_exception_debug();
> +    gen_exception_end_tb(ctx, EXCP_DEBUG);

The set of the pc is also redundant?


r~