| Message ID | 20220804115548.13024-7-anjo@rev.ng (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/hexagon: introduce idef-parser | expand |
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`
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)