Re: [PATCH 14/20] target/arm: Generalize inl_qrdmlah_* helper functions

[Peter Maydell]

On Sat, 15 Aug 2020 at 02:32, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Unify add/sub helpers and add a parameter for rounding.
> This will allow saturating non-rounding to reuse this code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

>  /* Signed saturating rounding doubling multiply-accumulate high half, 32-bit 
> */
> -static int32_t inl_qrdmlah_s32(int32_t src1, int32_t src2,
> -                               int32_t src3, uint32_t *sat)
> +static int32_t do_sqrdmlah_s(int32_t src1, int32_t src2, int32_t src3,
> +                             bool neg, bool round, uint32_t *sat)
>  {
>      /* Simplify similarly to int_qrdmlah_s16 above.  */
>      int64_t ret = (int64_t)src1 * src2;
> -    ret = ((int64_t)src3 << 31) + ret + (1 << 30);
> +    if (neg) {
> +        ret = -ret;
> +    }
> +    ret = ((int64_t)src3 << 31) + (round << 30);

Shouldn't this be "+=" as with the _h version earlier ?
(risu testing ought to catch this -- do we have a coverage hole?)

>      ret >>= 31;
> +
>      if (ret != (int32_t)ret) {
>          *sat = 1;
>          ret = (ret < 0 ? INT32_MIN : INT32_MAX);

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM