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
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 >
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
© 2016 - 2024 Red Hat, Inc.