Re: [PATCH 03/10] target/arm: Implement v8.1M conditional-select insns

[Richard Henderson <richard.henderson@linaro.org>]

On 10/12/20 8:37 AM, Peter Maydell wrote:
> v8.1M brings four new insns to M-profile:
>  * CSEL  : Rd = cond ? Rn : Rm
>  * CSINC : Rd = cond ? Rn : Rm+1
>  * CSINV : Rd = cond ? Rn : ~Rm
>  * CSNEG : Rd = cond ? Rn : -Rm
> 
> Implement these.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/t32.decode  |  3 +++
>  target/arm/translate.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> index 7069d821fde..d8454bd814e 100644
> --- a/target/arm/t32.decode
> +++ b/target/arm/t32.decode
> @@ -90,6 +90,9 @@ SBC_rrri         1110101 1011 . .... 0 ... .... .... ....     @s_rrr_shi
>  }
>  RSB_rrri         1110101 1110 . .... 0 ... .... .... ....     @s_rrr_shi
>  
> +# v8.1M CSEL and friends
> +CSEL             1110101 0010 1 rn:4 10 op:2 rd:4 fcond:4 rm:4
> +
>  # Data-processing (register-shifted register)
>  
>  MOV_rxrr         1111 1010 0 shty:2 s:1 rm:4 1111 rd:4 0000 rs:4 \
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index d34c1d351a6..a7923a31b56 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8224,6 +8224,61 @@ static bool trans_IT(DisasContext *s, arg_IT *a)
>      return true;
>  }
>  
> +/* v8.1M CSEL/CSINC/CSNEG/CSINV */
> +static bool trans_CSEL(DisasContext *s, arg_CSEL *a)
> +{
> +    TCGv_i32 rn, rm, zero;
> +    DisasCompare c;
> +
> +    if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
> +        return false;
> +    }
> +
> +    if (a->rd == 13 || a->rd == 15 || a->rn == 13 || a->fcond >= 14) {
> +        /* CONSTRAINED UNPREDICTABLE: we choose to UNDEF */
> +        return false;
> +    }

Missing check for rm != 13, which if I read the table properly would be an MVE
shift instruction.  (Irritatingly, there's a note for "See related encodings",
but there's no related encodings section.)

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~