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.hender...@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~
