On Thu, 21 Apr 2022 at 16:55, Richard Henderson <richard.hender...@linaro.org> wrote: > > Do as little work as possible within the macros. > Split out helper functions and pass in arguments instead. > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > target/nios2/translate.c | 215 +++++++++++++++++++++++++-------------- > 1 file changed, 141 insertions(+), 74 deletions(-) > > diff --git a/target/nios2/translate.c b/target/nios2/translate.c > index a3c63dbbbd..74672101ca 100644 > --- a/target/nios2/translate.c > +++ b/target/nios2/translate.c > @@ -71,6 +71,21 @@ typedef struct { > .a = extract32((code), 27, 5), \ > } > > +static target_ulong imm_unsigned(const InstrIType *i) > +{ > + return i->imm16.u; > +} > + > +static target_ulong imm_signed(const InstrIType *i) > +{ > + return i->imm16.s; > +} > + > +static target_ulong imm_shifted(const InstrIType *i) > +{ > + return i->imm16.u << 16; > +} > + > /* R-Type instruction parsing */ > typedef struct { > uint8_t op; > @@ -268,40 +283,62 @@ static void gen_bxx(DisasContext *dc, uint32_t code, > uint32_t flags) > } > > /* Comparison instructions */ > -#define gen_i_cmpxx(fname, op3) > \ > -static void (fname)(DisasContext *dc, uint32_t code, uint32_t flags) > \ > -{ > \ > - I_TYPE(instr, (code)); > \ > - tcg_gen_setcondi_tl(flags, cpu_R[instr.b], cpu_R[instr.a], (op3)); > \ > +static void do_i_cmpxx(DisasContext *dc, uint32_t insn, TCGCond cond, > + target_ulong (*imm)(const InstrIType *))
Can we have some typedefs if we're passing function pointers around, please? I think they're easier to read than having raw function-pointer type signatures in function prototypes. > +{ > + I_TYPE(instr, insn); > + > + if (likely(instr.b != R_ZERO)) { > + tcg_gen_setcondi_tl(cond, cpu_R[instr.b], > + load_gpr(dc, instr.a), imm(&instr)); > + } The old code didn't do this check against R_ZERO. > } > /* Math/logic instructions */ > -#define gen_r_math_logic(fname, insn, op3) \ > -static void (fname)(DisasContext *dc, uint32_t code, uint32_t flags) \ > -{ \ > - R_TYPE(instr, (code)); \ > - if (likely(instr.c != R_ZERO)) { \ > - tcg_gen_##insn(cpu_R[instr.c], load_gpr((dc), instr.a), (op3)); \ > - } \ > +static void do_ri_math_logic(DisasContext *dc, uint32_t insn, > + void (*fn)(TCGv, TCGv, int32_t)) > +{ > + R_TYPE(instr, insn); > + > + if (likely(instr.c != R_ZERO)) { > + fn(cpu_R[instr.c], load_gpr(dc, instr.a), instr.imm5); > + } > } > > -gen_r_math_logic(add, add_tl, load_gpr(dc, instr.b)) > -gen_r_math_logic(sub, sub_tl, load_gpr(dc, instr.b)) > -gen_r_math_logic(mul, mul_tl, load_gpr(dc, instr.b)) > +static void do_rr_math_logic(DisasContext *dc, uint32_t insn, > + void (*fn)(TCGv, TCGv, TCGv)) > +{ > + R_TYPE(instr, insn); > > -gen_r_math_logic(and, and_tl, load_gpr(dc, instr.b)) > -gen_r_math_logic(or, or_tl, load_gpr(dc, instr.b)) > -gen_r_math_logic(xor, xor_tl, load_gpr(dc, instr.b)) > -gen_r_math_logic(nor, nor_tl, load_gpr(dc, instr.b)) > - > -gen_r_math_logic(srai, sari_tl, instr.imm5) > -gen_r_math_logic(srli, shri_tl, instr.imm5) > -gen_r_math_logic(slli, shli_tl, instr.imm5) > -gen_r_math_logic(roli, rotli_tl, instr.imm5) > - > -#define gen_r_mul(fname, insn) \ > -static void (fname)(DisasContext *dc, uint32_t code, uint32_t flags) \ > -{ \ > - R_TYPE(instr, (code)); \ > - if (likely(instr.c != R_ZERO)) { \ > - TCGv t0 = tcg_temp_new(); \ > - tcg_gen_##insn(t0, cpu_R[instr.c], \ > - load_gpr(dc, instr.a), load_gpr(dc, instr.b)); \ > - tcg_temp_free(t0); \ > - } \ > + if (likely(instr.c != R_ZERO)) { > + fn(cpu_R[instr.c], load_gpr(dc, instr.a), load_gpr(dc, instr.b)); > + } > } > > -gen_r_mul(mulxss, muls2_tl) > -gen_r_mul(mulxuu, mulu2_tl) > -gen_r_mul(mulxsu, mulsu2_tl) > +#define gen_ri_math_logic(fname, insn) \ > + static void (fname)(DisasContext *dc, uint32_t code, uint32_t flags) \ > + { do_ri_math_logic(dc, code, tcg_gen_##insn##_tl); } > > -#define gen_r_shift_s(fname, insn) \ > -static void (fname)(DisasContext *dc, uint32_t code, uint32_t flags) \ > -{ \ > - R_TYPE(instr, (code)); \ > - if (likely(instr.c != R_ZERO)) { \ > - TCGv t0 = tcg_temp_new(); \ > - tcg_gen_andi_tl(t0, load_gpr((dc), instr.b), 31); \ > - tcg_gen_##insn(cpu_R[instr.c], load_gpr((dc), instr.a), t0); \ > - tcg_temp_free(t0); \ > - } \ > +#define gen_rr_math_logic(fname, insn) \ > + static void (fname)(DisasContext *dc, uint32_t code, uint32_t flags) \ > + { do_rr_math_logic(dc, code, tcg_gen_##insn##_tl); } git diff has made a bit of a pig's ear of this, interleaving the changes related to math_logic and the ones related to shift and mul. If you do these changes one insn group at a time rather than all in one patch I think the resulting diff should be easier to read. -- PMM