Re: [RFC PATCH] target/sh4: Fix SUBV opcode

[Philippe Mathieu-Daudé]

On 30/4/24 16:16, Paul Cercueil wrote:
> Hi Philippe,
>
> If I'm not mistaken, the overflow / underflow can be calculated like
> this:
>
> T = ((Rn ^ Rm) & (Result ^ Rn)) >> 31
>
> Looking at what Qemu does (before this patch), it was doing this:
> T = ((Rn ^ Rm) & (Result ^ Rm)) >> 31
>
> I changed line 936 to this, and overflow / underflow with SUBV now seem
> to work fine:
>
> tcg_gen_xor_i32(t1, t0, REG(B11_8));
>
> So a change from REG(B7_B4) to REG(B11_8).

Correct, thanks!

> Cheers,
> -Paul
>
> Le mardi 30 avril 2024 à 14:06 +0200, Philippe Mathieu-Daudé a écrit :
> > The documentation says:
> >
> >    SUBV Rm, Rn        Rn - Rm -> Rn, underflow -> T
> >
> > While correctly performing the substraction, the underflow
> > is not detected.
> >
> > While we can check the high xored bit for overflow, for
> > underflow we need to check the xored value is not negative.
> >
> > Cc: qemu-stable@nongnu.org
> > Fixes: ad8d25a11f ("target-sh4: implement addv and subv using TCG")
> > Reported-by: Paul Cercueil <paul@crapouillou.net>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2318
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   target/sh4/translate.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> > index 4a1dd0d1f4..1c48d8ebea 100644
> > --- a/target/sh4/translate.c
> > +++ b/target/sh4/translate.c
> > @@ -937,7 +937,7 @@ static void _decode_opc(DisasContext * ctx)
> >               t2 = tcg_temp_new();
> >               tcg_gen_xor_i32(t2, REG(B11_8), REG(B7_4));
> >               tcg_gen_and_i32(t1, t1, t2);
> > -            tcg_gen_shri_i32(cpu_sr_t, t1, 31);
> > +            tcg_gen_setcondi_i32(TCG_COND_GE, cpu_sr_t, t1, 0);
> >               tcg_gen_mov_i32(REG(B11_8), t0);
> >           }
> >           return;