* [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order
@ 2019-08-29 1:32 Richard Henderson
2019-08-29 7:49 ` Laurent Desnogues
2019-09-03 9:56 ` Peter Maydell
0 siblings, 2 replies; 3+ messages in thread
From: Richard Henderson @ 2019-08-29 1:32 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent.desnogues
The previous simplification got the order of operands to the
subtraction wrong. Since the 64-bit product is the subtrahend,
we must use a 64-bit subtract to properly compute the borrow
from the low-part of the product.
Fixes: 5f8cd06ebcf5 ("target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR")
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index cbe19b7a62..a0f7577f47 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8824,7 +8824,16 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
if (rd != 15) {
tmp3 = load_reg(s, rd);
if (insn & (1 << 6)) {
- tcg_gen_sub_i32(tmp, tmp, tmp3);
+ /*
+ * For SMMLS, we need a 64-bit subtract.
+ * Borrow caused by a non-zero multiplicand
+ * lowpart, and the correct result lowpart
+ * for rounding.
+ */
+ TCGv_i32 zero = tcg_const_i32(0);
+ tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3,
+ tmp2, tmp);
+ tcg_temp_free_i32(zero);
} else {
tcg_gen_add_i32(tmp, tmp, tmp3);
}
@@ -10068,7 +10077,14 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
if (insn & (1 << 20)) {
tcg_gen_add_i32(tmp, tmp, tmp3);
} else {
- tcg_gen_sub_i32(tmp, tmp, tmp3);
+ /*
+ * For SMMLS, we need a 64-bit subtract.
+ * Borrow caused by a non-zero multiplicand lowpart,
+ * and the correct result lowpart for rounding.
+ */
+ TCGv_i32 zero = tcg_const_i32(0);
+ tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3, tmp2, tmp);
+ tcg_temp_free_i32(zero);
}
tcg_temp_free_i32(tmp3);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order
2019-08-29 1:32 [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order Richard Henderson
@ 2019-08-29 7:49 ` Laurent Desnogues
2019-09-03 9:56 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Laurent Desnogues @ 2019-08-29 7:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel@nongnu.org
Hi,
On Thu, Aug 29, 2019 at 3:33 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The previous simplification got the order of operands to the
> subtraction wrong. Since the 64-bit product is the subtrahend,
> we must use a 64-bit subtract to properly compute the borrow
> from the low-part of the product.
>
> Fixes: 5f8cd06ebcf5 ("target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR")
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index cbe19b7a62..a0f7577f47 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8824,7 +8824,16 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
> if (rd != 15) {
> tmp3 = load_reg(s, rd);
> if (insn & (1 << 6)) {
> - tcg_gen_sub_i32(tmp, tmp, tmp3);
> + /*
> + * For SMMLS, we need a 64-bit subtract.
> + * Borrow caused by a non-zero multiplicand
> + * lowpart, and the correct result lowpart
> + * for rounding.
> + */
> + TCGv_i32 zero = tcg_const_i32(0);
> + tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3,
> + tmp2, tmp);
> + tcg_temp_free_i32(zero);
> } else {
> tcg_gen_add_i32(tmp, tmp, tmp3);
> }
> @@ -10068,7 +10077,14 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
> if (insn & (1 << 20)) {
> tcg_gen_add_i32(tmp, tmp, tmp3);
> } else {
> - tcg_gen_sub_i32(tmp, tmp, tmp3);
> + /*
> + * For SMMLS, we need a 64-bit subtract.
> + * Borrow caused by a non-zero multiplicand lowpart,
> + * and the correct result lowpart for rounding.
> + */
> + TCGv_i32 zero = tcg_const_i32(0);
> + tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3, tmp2, tmp);
> + tcg_temp_free_i32(zero);
> }
> tcg_temp_free_i32(tmp3);
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order
2019-08-29 1:32 [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order Richard Henderson
2019-08-29 7:49 ` Laurent Desnogues
@ 2019-09-03 9:56 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2019-09-03 9:56 UTC (permalink / raw)
To: Richard Henderson; +Cc: Laurent Desnogues, QEMU Developers
On Thu, 29 Aug 2019 at 02:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The previous simplification got the order of operands to the
> subtraction wrong. Since the 64-bit product is the subtrahend,
> we must use a 64-bit subtract to properly compute the borrow
> from the low-part of the product.
>
> Fixes: 5f8cd06ebcf5 ("target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR")
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-03 9:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-29 1:32 [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order Richard Henderson
2019-08-29 7:49 ` Laurent Desnogues
2019-09-03 9:56 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).