Re: [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order

[Peter Maydell]

On Thu, 29 Aug 2019 at 02:34, Richard Henderson
 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 
> Signed-off-by: Richard Henderson 



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order

[Laurent Desnogues]

Hi,

On Thu, Aug 29, 2019 at 3:33 AM Richard Henderson
 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 
> Signed-off-by: Richard Henderson 

Tested-by: Laurent Desnogues 

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
>

[Qemu-devel] [PATCH] target/arm: Fix SMMLS argument order

[Richard Henderson]

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 
Signed-off-by: Richard Henderson 
---
 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