Re: [Qemu-devel] [PATCH] target-arm: Set Q bit for overflow in SMUAD and SMLAD

[Aurelien Jarno]

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 <address@hidden>
> ---
>  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
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net