diff mbox

target-arm: Set Q bit for overflow in SMUAD and SMLAD

Message ID 1299838198-21621-1-git-send-email-peter.maydell@linaro.org
State Accepted
Commit e1d177b922f52569e900e96d611caa09655bdec9
Headers show

Commit Message

Peter Maydell March 11, 2011, 10:09 a.m. UTC
SMUAD and SMLAD are supposed to set the Q bit if the addition of
the two 16x16 multiply products and optional accumulator overflows
considered as a signed value. However we were only doing this check
for the addition of the accumulator, not when adding the products,
with the effect that we were mishandling the edge case where
both inputs are 0x80008000.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

Comments

Aurelien Jarno March 22, 2011, 6:58 a.m. UTC | #1
On Fri, Mar 11, 2011 at 10:09:58AM +0000, Peter Maydell wrote:
> SMUAD and SMLAD are supposed to set the Q bit if the addition of
> the two 16x16 multiply products and optional accumulator overflows
> considered as a signed value. However we were only doing this check
> for the addition of the accumulator, not when adding the products,
> with the effect that we were mishandling the edge case where
> both inputs are 0x80008000.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 062de5e..8f7c461 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7038,11 +7038,15 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                          if (insn & (1 << 5))
>                              gen_swap_half(tmp2);
>                          gen_smul_dual(tmp, tmp2);
> -                        /* This addition cannot overflow.  */
>                          if (insn & (1 << 6)) {
> +                            /* This subtraction cannot overflow. */
>                              tcg_gen_sub_i32(tmp, tmp, tmp2);
>                          } else {
> -                            tcg_gen_add_i32(tmp, tmp, tmp2);
> +                            /* This addition cannot overflow 32 bits;
> +                             * however it may overflow considered as a signed
> +                             * operation, in which case we must set the Q flag.
> +                             */
> +                            gen_helper_add_setq(tmp, tmp, tmp2);
>                          }
>                          tcg_temp_free_i32(tmp2);
>                          if (insn & (1 << 22)) {
> @@ -7860,11 +7864,15 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                  if (op)
>                      gen_swap_half(tmp2);
>                  gen_smul_dual(tmp, tmp2);
> -                /* This addition cannot overflow.  */
>                  if (insn & (1 << 22)) {
> +                    /* This subtraction cannot overflow. */
>                      tcg_gen_sub_i32(tmp, tmp, tmp2);
>                  } else {
> -                    tcg_gen_add_i32(tmp, tmp, tmp2);
> +                    /* This addition cannot overflow 32 bits;
> +                     * however it may overflow considered as a signed
> +                     * operation, in which case we must set the Q flag.
> +                     */
> +                    gen_helper_add_setq(tmp, tmp, tmp2);
>                  }
>                  tcg_temp_free_i32(tmp2);
>                  if (rs != 15)
> -- 
> 1.7.1
> 
> 
>
diff mbox

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 062de5e..8f7c461 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7038,11 +7038,15 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                         if (insn & (1 << 5))
                             gen_swap_half(tmp2);
                         gen_smul_dual(tmp, tmp2);
-                        /* This addition cannot overflow.  */
                         if (insn & (1 << 6)) {
+                            /* This subtraction cannot overflow. */
                             tcg_gen_sub_i32(tmp, tmp, tmp2);
                         } else {
-                            tcg_gen_add_i32(tmp, tmp, tmp2);
+                            /* This addition cannot overflow 32 bits;
+                             * however it may overflow considered as a signed
+                             * operation, in which case we must set the Q flag.
+                             */
+                            gen_helper_add_setq(tmp, tmp, tmp2);
                         }
                         tcg_temp_free_i32(tmp2);
                         if (insn & (1 << 22)) {
@@ -7860,11 +7864,15 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 if (op)
                     gen_swap_half(tmp2);
                 gen_smul_dual(tmp, tmp2);
-                /* This addition cannot overflow.  */
                 if (insn & (1 << 22)) {
+                    /* This subtraction cannot overflow. */
                     tcg_gen_sub_i32(tmp, tmp, tmp2);
                 } else {
-                    tcg_gen_add_i32(tmp, tmp, tmp2);
+                    /* This addition cannot overflow 32 bits;
+                     * however it may overflow considered as a signed
+                     * operation, in which case we must set the Q flag.
+                     */
+                    gen_helper_add_setq(tmp, tmp, tmp2);
                 }
                 tcg_temp_free_i32(tmp2);
                 if (rs != 15)