> -----Original Message----- > From: Richard Henderson <richard.hender...@linaro.org> > Sent: Friday, August 28, 2020 7:49 PM > To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 28/34] Hexagon (target/hexagon) TCG > generation helpers > > On 8/18/20 8:50 AM, Taylor Simpson wrote: > > Helpers for reading and writing registers > > Helpers for load-locked/store-conditional > > > > Signed-off-by: Taylor Simpson <tsimp...@quicinc.com> > > --- > > target/hexagon/genptr_helpers.h | 244 > ++++++++++++++++++++++++++++++++++++++++ > > target/hexagon/op_helper.c | 18 +++ > > 2 files changed, 262 insertions(+) > > create mode 100644 target/hexagon/genptr_helpers.h > > > > diff --git a/target/hexagon/genptr_helpers.h > b/target/hexagon/genptr_helpers.h > > new file mode 100644 > > index 0000000..ffcb1e3 > > --- /dev/null > > +++ b/target/hexagon/genptr_helpers.h > > @@ -0,0 +1,244 @@ > > + > > +static inline void gen_log_reg_write(int rnum, TCGv val, int slot, > > + int is_predicated) > > These are quite large. Why are they marked inline?
Since this is a header file, it prevents the compiler from complaining when they aren't used. > > > + /* Low word */ > > + tcg_gen_extrl_i64_i32(val32, val); > > + tcg_gen_mov_tl(hex_new_value[rnum], val32); > > Why are you extracting into a temporary? > This could be done with > > tcg_gen_extr_i64_i32(hex_new_value[rnum], > hex_new_value[rnum + 1], val); OK > > +static inline void gen_read_p3_0(TCGv control_reg) > > +{ > > + TCGv pval = tcg_temp_new(); > > + int i; > > + tcg_gen_movi_tl(control_reg, 0); > > + for (i = NUM_PREGS - 1; i >= 0; i--) { > > + tcg_gen_shli_tl(control_reg, control_reg, 8); > > + tcg_gen_andi_tl(pval, hex_pred[i], 0xff); > > + tcg_gen_or_tl(control_reg, control_reg, pval); > > tcg_gen_deposit_tl(control_reg, control_reg, > hex_pred[i], i * 8, 8); OK > > + for (i = 0; i < NUM_PREGS; i++) { > > + tcg_gen_andi_tl(pred_val, control_reg, 0xff); > > + tcg_gen_mov_tl(hex_pred[i], pred_val); > > + tcg_gen_shri_tl(control_reg, control_reg, 8); > > tcg_gen_extract_tl(hex_pred[i], control_reg, i * 8, 8); OK > > +static inline void log_store32(CPUHexagonState *env, target_ulong addr, > > + int32_t val, int width, int slot) > > +{ > > + HEX_DEBUG_LOG("log_store%d(0x%x, %d [0x%x])\n", width, addr, val, > val); > > + env->mem_log_stores[slot].va = addr; > > + env->mem_log_stores[slot].width = width; > > + env->mem_log_stores[slot].data32 = val; > > +} > > + > > +static inline void log_store64(CPUHexagonState *env, target_ulong addr, > > + int64_t val, int width, int slot) > > +{ > > + HEX_DEBUG_LOG("log_store%d(0x%x, %ld [0x%lx])\n", width, addr, > val, val); > > + env->mem_log_stores[slot].va = addr; > > + env->mem_log_stores[slot].width = width; > > + env->mem_log_stores[slot].data64 = val; > > +} > > ... or fold this re-addition back into where it was accidentally removed. ;-) Could you elaborate?