Re: [PATCH 9/9] target/s390x: Use Int128 for passing float128

[Ilya Leoshkevich]

On Wed, Nov 02, 2022 at 08:47:24PM +1100, Richard Henderson wrote:
> On 11/2/22 20:38, Ilya Leoshkevich wrote:
> > On Fri, Oct 21, 2022 at 05:30:06PM +1000, Richard Henderson wrote:
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > ---
> > >   target/s390x/helper.h          | 32 ++++++-------
> > >   target/s390x/tcg/fpu_helper.c  | 88 ++++++++++++++--------------------
> > >   target/s390x/tcg/translate.c   | 76 ++++++++++++++++++++---------
> > >   target/s390x/tcg/insn-data.def | 30 ++++++------
> > >   4 files changed, 121 insertions(+), 105 deletions(-)

...

> > Hi,
> > 
> > I ran valgrind's testsuite with this patch, and their bpf-4 test
> > triggered an assertion in the
> > 
> >      (insn->spec & SPEC_r2_f128 && !is_fp_pair(get_field(s, r2)))
> > 
> > condition. The following fixup helped:
> > 
> > 
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -5771,14 +5771,14 @@ static void in1_x1(DisasContext *s, DisasOps *o)
> >   {
> >       o->in1_128 = load_freg_128(get_field(s, r1));
> >   }
> > -#define SPEC_in1_x1 SPEC_r2_f128
> > +#define SPEC_in1_x1 SPEC_r1_f128
> 
> Looks right, thanks.
> 
> >   /* Load the high double word of an extended (128-bit) format FP number */
> >   static void in1_x2h(DisasContext *s, DisasOps *o)
> >   {
> >       o->in1 = load_freg(get_field(s, r2));
> >   }
> > -#define SPEC_in1_x2h SPEC_r2_f128
> > +#define SPEC_in1_x2h SPEC_r1_f128
> 
> This looks wrong.

Oh, right - we do get_field(r2) here.
Only the first hunk is necessary.

> r~