Re: [PATCH 3/9] target/Hexagon: Finish conversion to tcg_gen_qemu_{ld,st}_*

[Richard Henderson]

On 5/2/23 16:27, Taylor Simpson wrote:
> > -----Original Message-----
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Sent: Tuesday, May 2, 2023 8:58 AM
> > To: qemu-devel@nongnu.org
> > Cc: mrolnik@gmail.com; edgar.iglesias@gmail.com; Taylor Simpson
> > <tsimpson@quicinc.com>; ale@rev.ng; anjo@rev.ng; laurent@vivier.eu;
> > philmd@linaro.org; jiaxun.yang@flygoat.com; david@redhat.com;
> > iii@linux.ibm.com; thuth@redhat.com; mark.cave-ayland@ilande.co.uk;
> > atar4qemu@gmail.com; jcmvbkbc@gmail.com
> > Subject: [PATCH 3/9] target/Hexagon: Finish conversion to
> > tcg_gen_qemu_{ld,st}_*
> >
> > Convert away from the old interface with the implicit MemOp argument.
> > Importantly, this removes some incorrect casts generated by idef-parser's
> > gen_load().
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   target/hexagon/macros.h                     | 14 ++++-----
> >   target/hexagon/genptr.c                     |  8 +++---
> >   target/hexagon/idef-parser/parser-helpers.c | 28 +++++++++---------
> >   target/hexagon/translate.c                  | 32 ++++++++++-----------
> >   4 files changed, 40 insertions(+), 42 deletions(-)
> >
> > diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c index
> > 502c85ae35..244063b1d2 100644
> > --- a/target/hexagon/genptr.c
> > +++ b/target/hexagon/genptr.c
> > @@ -320,14 +320,14 @@ void gen_set_byte_i64(int N, TCGv_i64 result, TCGv
> > src)
> >
> >   static void gen_return(DisasContext *ctx, TCGv_i64 dst, TCGv src) @@ -
> > 1019,7 +1019,7 @@ static void gen_vreg_load(DisasContext *ctx, intptr_t
> > dstoff, TCGv src,
> >           tcg_gen_andi_tl(src, src, ~((int32_t)sizeof(MMVector) - 1));
> >       }
> >       for (int i = 0; i < sizeof(MMVector) / 8; i++) {
> > -        tcg_gen_qemu_ld64(tmp, src, ctx->mem_idx);
> > +        tcg_gen_qemu_ld_i64(tmp, src, ctx->mem_idx, MO_TEUQ);
> >           tcg_gen_addi_tl(src, src, 8);
> >           tcg_gen_st_i64(tmp, cpu_env, dstoff + i * 8);
>
> Did you intend to leave the tcg_gen_st_i64 alone or should that be converted to 
> tcg_gen_qemu_st64.
>
> There's a tcg_gen_st8_i64 in vec_to_qvec.  Does that need to be converted?

No, those are host stores to env, not guest stores to guest memory.
Notice the lack of "qemu" in the function names.

> I'm curious if there's a better way to do a vector load (e.g., with 
> tcg_gen_gvec_<something>) than a loop that does 8 bytes at a time.

The best you can do at the moment is tcg_gen_qemu_ld_i128.
But there's no gvec variant to load arbitrary vector lengths.


r~