Re: [Qemu-arm] [PATCH v2 03/67] target/arm: Add SVE decode skeleton

[Peter Maydell]

On 17 February 2018 at 18:22, Richard Henderson
<address@hidden> wrote:
> Including only 4, as-yet unimplemented, instruction patterns
> so that the whole thing compiles.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate-a64.c | 11 +++++++-
>  target/arm/translate-sve.c | 63 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  .gitignore                 |  1 +
>  target/arm/Makefile.objs   | 10 ++++++++
>  target/arm/sve.decode      | 45 +++++++++++++++++++++++++++++++++
>  5 files changed, 129 insertions(+), 1 deletion(-)
>  create mode 100644 target/arm/translate-sve.c
>  create mode 100644 target/arm/sve.decode
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index e0e7ebf68c..a50fef98af 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -12772,9 +12772,18 @@ static void disas_a64_insn(CPUARMState *env, 
> DisasContext *s)
>      s->fp_access_checked = false;
>
>      switch (extract32(insn, 25, 4)) {
> -    case 0x0: case 0x1: case 0x2: case 0x3: /* UNALLOCATED */
> +    case 0x0: case 0x1: case 0x3: /* UNALLOCATED */
>          unallocated_encoding(s);
>          break;
> +    case 0x2:
> +        if (!arm_dc_feature(s, ARM_FEATURE_SVE)) {
> +            unallocated_encoding(s);
> +        } else if (!sve_access_check(s) || !fp_access_check(s)) {
> +            /* exception raised */
> +        } else if (!disas_sve(s, insn)) {
> +            unallocated_encoding(s);
> +        }
> +        break;

I realized while working through the rest of the series that this is
too early to do the sve_access_check() and fp_access_check(). Those
only apply to instructions which actually exist, so we mustn't
do the checks until after we've dealt with all the unallocated_encoding()
cases. I think you need to push them down inside disas_sve() somehow.
See also my comments on patch 8.

(We get this wrong for current AArch32 VFP and Neon, but correct
for all of AArch64.)

thanks
-- PMM