| Message ID | 20190829013258.16102-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Fix SMMLS argument order | expand |
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
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); }
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(-) -- 2.17.1