From: Paolo Montesel <babush@rev.ng>

Signed-off-by: Alessandro Di Federico <ale@rev.ng>
Signed-off-by: Paolo Montesel <babush@rev.ng>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/translate.c | 3 ++-
 target/hexagon/translate.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index d4fc92f7e9..e3e250fd4f 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -741,11 +741,12 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
     if (decode_packet(nwords, words, &pkt, false) > 0) {
         HEX_DEBUG_PRINT_PKT(&pkt);
         gen_start_packet(ctx, &pkt);
+        ctx->npc = ctx->base.pc_next + pkt.encod_pkt_size_in_bytes;
         for (i = 0; i < pkt.num_insns; i++) {
             gen_insn(env, ctx, &pkt.insn[i], &pkt);
         }
         gen_commit_packet(env, ctx, &pkt);
-        ctx->base.pc_next += pkt.encod_pkt_size_in_bytes;
+        ctx->base.pc_next = ctx->npc;
     } else {
         gen_exception_end_tb(ctx, HEX_EXCP_INVALID_PACKET);
     }
diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index a245172827..494471548e 100644
--- a/target/hexagon/translate.h
+++ b/target/hexagon/translate.h
@@ -53,6 +53,7 @@ typedef struct DisasContext {
     bool qreg_is_predicated[NUM_QREGS];
     int qreg_log_idx;
     bool pre_commit;
+    uint32_t npc;
 } DisasContext;
 
 static inline void ctx_log_reg_write(DisasContext *ctx, int rnum)
-- 
2.37.0

On 4/8/22 13:55, Anton Johansson via wrote:
> From: Paolo Montesel <babush@rev.ng>

Missing the rationale. "The idef-parser will use it with IMM_NPC".

But I feel I'm missing something, what is the diff between IMM_PC/IMM_NPC?

> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> Signed-off-by: Paolo Montesel <babush@rev.ng>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
>   target/hexagon/translate.c | 3 ++-
>   target/hexagon/translate.h | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
> index d4fc92f7e9..e3e250fd4f 100644
> --- a/target/hexagon/translate.c
> +++ b/target/hexagon/translate.c
> @@ -741,11 +741,12 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
>       if (decode_packet(nwords, words, &pkt, false) > 0) {
>           HEX_DEBUG_PRINT_PKT(&pkt);
>           gen_start_packet(ctx, &pkt);
> +        ctx->npc = ctx->base.pc_next + pkt.encod_pkt_size_in_bytes;
>           for (i = 0; i < pkt.num_insns; i++) {
>               gen_insn(env, ctx, &pkt.insn[i], &pkt);
>           }
>           gen_commit_packet(env, ctx, &pkt);
> -        ctx->base.pc_next += pkt.encod_pkt_size_in_bytes;
> +        ctx->base.pc_next = ctx->npc;
>       } else {
>           gen_exception_end_tb(ctx, HEX_EXCP_INVALID_PACKET);
>       }
> diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
> index a245172827..494471548e 100644
> --- a/target/hexagon/translate.h
> +++ b/target/hexagon/translate.h
> @@ -53,6 +53,7 @@ typedef struct DisasContext {
>       bool qreg_is_predicated[NUM_QREGS];
>       int qreg_log_idx;
>       bool pre_commit;
> +    uint32_t npc;

And why not use target_ulong?

>   } DisasContext;
>   
>   static inline void ctx_log_reg_write(DisasContext *ctx, int rnum)

> Missing the rationale. "The idef-parser will use it with IMM_NPC".
>
> But I feel I'm missing something, what is the diff between 
> IMM_PC/IMM_NPC?
>
I'll try to clarify.

Firstly, why do we need this patch? Hexagon intructions need access to
both the current pc and the next pc, for the generated helper functions
this is done through env->gpr[HEX_REG_PC] and `env->next_PC`. However
for the TCG code generated by idef-parser we much prefer reading pc and
npc from `DisasContext` as these are constant during translation time,
we can then "constant fold" by emitting C code for operations on pc and
npc instead of TCG code.

Secondly, what's IMM_NPC and IMM_PC? This is internal to the parser, but
refers to types of immediate values `HexImm`, an immediate with type
IMM_NPC can then easily be emitted as `ctx->npc`, similarly IMM_PC gets
emitted as `ctx->base.pc_next`.

> And why not use target_ulong?

Good idea, I'll change it to `target_ulong`

-- 
Anton Johansson,
rev.ng Labs Srl.