Re: [Qemu-devel] [Qemu-arm] [PATCH v2 44/67] target/arm: Implement SVE Memory Contiguous Load Group

[Peter Maydell]

On 17 February 2018 at 18:23, Richard Henderson
<address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>

Commit message should mention significant missing things
like first-fault/non-fault handling. (In general I would prefer
not to see so many patches which all have one-liner commit
messages.)

> +static void trans_LDFF1_zprr(DisasContext *s, arg_rprr_load *a, uint32_t 
> insn)
> +{
> +    /* FIXME */
> +    trans_LD_zprr(s, a, insn);
> +}
> +
> +static void trans_LDNF1_zpri(DisasContext *s, arg_rpri_load *a, uint32_t 
> insn)
> +{
> +    /* FIXME */
> +    trans_LD_zpri(s, a, insn);
> +}

What are these FIXMEs for? Either they should be fixed, or expanded
into longer comments describing what needs fixing. I assume it is
the missing non-fault/first-fault behaviour...

> diff --git a/target/arm/sve.decode b/target/arm/sve.decode
> index 42d14994a1..d2b3869c58 100644
> --- a/target/arm/sve.decode
> +++ b/target/arm/sve.decode
> @@ -42,9 +42,12 @@
>  %tszimm16_shl  22:2 16:5 !function=tszimm_shl
>
>  # Signed 8-bit immediate, optionally shifted left by 8.
> -%sh8_i8s               5:9 !function=expand_imm_sh8s
> +%sh8_i8s       5:9 !function=expand_imm_sh8s
>  # Unsigned 8-bit immediate, optionally shifted left by 8.
> -%sh8_i8u               5:9 !function=expand_imm_sh8u
> +%sh8_i8u       5:9 !function=expand_imm_sh8u


More changes that should be squashed into earlier patch.


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

thanks
-- PMM