| Message ID | 20240430120650.70539-1-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] target/sh4: Fix SUBV opcode | expand |
Hi Philippe, 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. This fix still does not work properly; it will incorrectly set the underflow bit when the sign changes from positive to negative. e.g. Rn == 0 and Rm == 2, the result will be Rn == -2, without any underflow. Cheers, -Paul > > 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;
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). 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;
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; >
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;
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(-)