> -----Original Message----- > From: Richard Henderson <richard.hender...@linaro.org> > Sent: Sunday, February 14, 2021 7:04 PM > To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org > Cc: phi...@redhat.com; alex.ben...@linaro.org; laur...@vivier.eu; > a...@rev.ng; Brian Cain <bc...@quicinc.com> > Subject: Re: [PATCH v8 29/35] Hexagon (target/hexagon) translation > > On 2/7/21 9:46 PM, Taylor Simpson wrote: > > +static inline void ctx_log_reg_write(DisasContext *ctx, int rnum) > > Drop the inline markup throughout.
I can go through the code and remove unnecessary inline's. However, these particular inline's are needed because this is a header file. If we remove the inline and the header gets included in a .c file that doesn't use the function, we get a "defined but not used" error. Also, we need to keep the inline's in genptr.c to avoid the same error when we switch an instruction between the fGEN_TCG and helper implementations (and the idef-parser in the future). Also, there is one function that needs to be inline for performance reasons. I'll add a comment for that one. > > + words[nwords] = cpu_ldl_code(env, > > + ctx->base.pc_next + nwords * > > sizeof(uint32_t)); > > translate_ldl, so that a plugin has access to the packet data. (Note that > pkt_crosses_page is fine, because that's read-ahead, not reads for the > current > packet.) OK > > Fold this to a simple function call: > > static void gen_check_store_width(...) > { > if (HEX_DEBUG) { > .... > } > } OK > > +#if HEX_DEBUG > > + /* When debugging, only put one packet per TB */ > > + ctx->base.is_jmp = DISAS_TOO_MANY; > > +#endif > > Why? You can always add -singlestep to the command-line. OK > > + case DISAS_NORETURN: > > + gen_exec_counters(ctx); > > + tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], hex_next_PC); > > + if (ctx->base.singlestep_enabled) { > > + gen_exception_debug(); > > + } else { > > + tcg_gen_exit_tb(NULL, 0); > > + } > > DISAS_NORETURN says that we have *already* exited the TB. None of the > code you > emit here will be reachable. Isn't this called before the TB ends? Here's the code in translator.c /* Emit code to exit the TB, as indicated by db->is_jmp. */ ops->tb_stop(db, cpu); gen_tb_end(db->tb, db->num_insns - bp_insn); Thanks, Taylor