> -----Original Message----- > From: Richard Henderson <richard.hender...@linaro.org> > Sent: Sunday, April 18, 2021 4:31 PM > To: Alessandro Di Federico <ale.q...@rev.ng>; qemu-devel@nongnu.org > Cc: Taylor Simpson <tsimp...@quicinc.com>; Brian Cain > <bc...@quicinc.com>; bab...@rev.ng; ni...@rev.ng; phi...@redhat.com; > Alessandro Di Federico <a...@rev.ng> > Subject: Re: [PATCH v4 06/12] target/hexagon: introduce new helper > functions > > On 4/15/21 9:34 AM, Alessandro Di Federico wrote: > > +void gen_store32(TCGv vaddr, TCGv src, tcg_target_long width, unsigned > slot) > > +{ > > + tcg_gen_mov_tl(hex_store_addr[slot], vaddr); > > + tcg_gen_movi_tl(hex_store_width[slot], width); > > + tcg_gen_mov_tl(hex_store_val32[slot], src); > > +} > > + > > +void gen_store1(TCGv_env cpu_env, TCGv vaddr, TCGv src, DisasContext > *ctx, > > + unsigned slot) > > +{ > > + gen_store32(vaddr, src, 1, slot); > > + ctx->store_width[slot] = 1; > > +} > > Why is store_width here and not in gen_store32? > Do you really need so many helpers here, as opposed to making use of > MemOp?
These are included in this patch https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01355.html which hasn't been merged yet. > > > +void gen_sat_i32_ext(TCGv ovfl, TCGv dest, TCGv source, int width) > > +{ > > + gen_sat_i32(dest, source, width); > > + TCGv zero = tcg_const_i32(0); > > + TCGv one = tcg_const_i32(1); > > + tcg_gen_movcond_i32(TCG_COND_NE, ovfl, source, dest, one, zero); > > (source != dest ? 1 : 0) -> (source != dest). > > Therefore, tcg_gen_setcond_i32. > > Or did you intend > > ovfl = (source != dest ? 1 : ovfl)? > > which is probably still better as > > tcg_gen_setcond_tl(TCG_COND_NE, tmp, source,dest); > tcg_gen_or_tl(ovfl, ovfl, tmp); > > > +void gen_fbrev(TCGv result, TCGv src) > > +{ > > + TCGv lo = tcg_temp_new(); > > + TCGv tmp1 = tcg_temp_new(); > > + TCGv tmp2 = tcg_temp_new(); > > + > > + /* Bit reversal of low 16 bits */ > > + tcg_gen_extract_tl(lo, src, 0, 16); > > + tcg_gen_andi_tl(tmp1, lo, 0xaaaa); > > + tcg_gen_shri_tl(tmp1, tmp1, 1); > > + tcg_gen_andi_tl(tmp2, lo, 0x5555); > > + tcg_gen_shli_tl(tmp2, tmp2, 1); > > + tcg_gen_or_tl(lo, tmp1, tmp2); > > + tcg_gen_andi_tl(tmp1, lo, 0xcccc); > > + tcg_gen_shri_tl(tmp1, tmp1, 2); > > + tcg_gen_andi_tl(tmp2, lo, 0x3333); > > + tcg_gen_shli_tl(tmp2, tmp2, 2); > > + tcg_gen_or_tl(lo, tmp1, tmp2); > > + tcg_gen_andi_tl(tmp1, lo, 0xf0f0); > > + tcg_gen_shri_tl(tmp1, tmp1, 4); > > + tcg_gen_andi_tl(tmp2, lo, 0x0f0f); > > + tcg_gen_shli_tl(tmp2, tmp2, 4); > > + tcg_gen_or_tl(lo, tmp1, tmp2); > > + tcg_gen_bswap16_tl(lo, lo); > > + > > + /* Final tweaks */ > > + tcg_gen_deposit_tl(result, src, lo, 0, 16); > > + tcg_gen_or_tl(result, result, lo); > > + > > + tcg_temp_free(lo); > > + tcg_temp_free(tmp1); > > + tcg_temp_free(tmp2); > > +} > > Coordinate with Taylor. > https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg10007.html Once this patch series is merged, many load/store instructions will have manual overrides. I can easily provide overrides for the remainder. Then, we could skip them in the idef-parser. At a minimum, you should skip the ones that are part of that series - circular addressing https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01355.html - bit reverse addressing https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01354.html - load and unpack bytes https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01353.html - load into shifted register https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01359.html Alessandro, what do you think? Thanks, Taylor