Re: [Qemu-devel] [Qemu-arm] [PATCH v2 11/67] target/arm: Implement SVE bitwise shift by immediate (predicated)

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
<address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/helper-sve.h    |  25 +++++
>  target/arm/sve_helper.c    | 265 
> +++++++++++++++++++++++++++++++++++++++++++++
>  target/arm/translate-sve.c | 128 ++++++++++++++++++++++
>  target/arm/sve.decode      |  29 ++++-
>  4 files changed, 445 insertions(+), 2 deletions(-)

>
> +/*
> + * Helpers for extracting complex instruction fields.
> + */
> +
> +/* See e.g. ASL (immediate, predicated).

Typo for "ASR", I guess ?

> + * Returns -1 for unallocated encoding; diagnose later.
> + */
> +static int tszimm_esz(int x)
> +{
> +    x >>= 3;  /* discard imm3 */
> +    return 31 - clz32(x);
> +}
> +
> +static int tszimm_shr(int x)
> +{
> +    return (16 << tszimm_esz(x)) - x;
> +}
> +
> +/* See e.g. LSL (immediate, predicated).  */
> +static int tszimm_shl(int x)
> +{
> +    return x - (8 << tszimm_esz(x));
> +}

> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -22,12 +22,20 @@
>  ###########################################################################
>  # Named fields.  These are primarily for disjoint fields.
>
> +%imm6_22_5     22:1 5:5
>  %imm9_16_10    16:s6 10:3
>  %preg4_5       5:4
>
> +# A combination of tsz:imm3 -- extract esize.
> +%tszimm_esz    22:2 5:5 !function=tszimm_esz
> +# A combination of tsz:imm3 -- extract (2 * esize) - (tsz:imm3)
> +%tszimm_shr    22:2 5:5 !function=tszimm_shr
> +# A combination of tsz:imm3 -- extract (tsz:imm3) - esize
> +%tszimm_shl    22:2 5:5 !function=tszimm_shl
> +
>  # Either a copy of rd (at bit 0), or a different source
>  # as propagated via the MOVPRFX instruction.
> -%reg_movprfx           0:5
> +%reg_movprfx   0:5

Squash into relevant previous patch.

>  ###########################################################################
>  # Named attribute sets.  These are used to make nice(er) names
> @@ -40,7 +48,7 @@
>  &rpr_esz       rd pg rn esz
>  &rprr_s                rd pg rn rm s
>  &rprr_esz      rd pg rn rm esz
> -
> +&rpri_esz      rd pg rn imm esz

Should either not delete the blank line, or don't add it in the first place.

>  &ptrue         rd esz pat s
>
>  ###########################################################################
> @@ -68,6 +76,11 @@
>  # One register operand, with governing predicate, vector element size
>  @rd_pg_rn      ........ esz:2 ... ... ... pg:3 rn:5 rd:5       &rpr_esz
>
> +# Two register operand, one immediate operand, with predicate,
> +# element size encoded as TSZHL.  User must fill in imm.
> address@hidden ........ .. ... ... ... pg:3 ..... rd:5 \
> +               &rpri_esz rn=%reg_movprfx esz=%tszimm_esz
> +
>  # Basic Load/Store with 9-bit immediate offset
>  @pd_rn_i9      ........ ........ ...... rn:5 . rd:4    \
>                 &rri imm=%imm9_16_10
> @@ -126,6 +139,18 @@ UMAXV              00000100 .. 001 001 001 ... ..... 
> .....         @rd_pg_rn
>  SMINV          00000100 .. 001 010 001 ... ..... .....         @rd_pg_rn
>  UMINV          00000100 .. 001 011 001 ... ..... .....         @rd_pg_rn
>
> +### SVE Shift by Immediate - Predicated Group
> +
> +# SVE bitwise shift by immediate (predicated)
> +ASR_zpzi       00000100 .. 000 000 100 ... .. ... ..... \
> +               @rdn_pg_tszimm imm=%tszimm_shr
> +LSR_zpzi       00000100 .. 000 001 100 ... .. ... ..... \
> +               @rdn_pg_tszimm imm=%tszimm_shr
> +LSL_zpzi       00000100 .. 000 011 100 ... .. ... ..... \
> +               @rdn_pg_tszimm imm=%tszimm_shl
> +ASRD           00000100 .. 000 100 100 ... .. ... ..... \
> +               @rdn_pg_tszimm imm=%tszimm_shr
> +
>  ### SVE Logical - Unpredicated Group
>
>  # SVE bitwise logical operations (unpredicated)
> --
> 2.14.3

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM