Re: [Qemu-devel] [PATCH v5 13/35] target/arm: Implement SVE gather loads

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

On 06/25/2018 09:55 AM, Peter Maydell wrote:
>> +static bool trans_LD1_zprz(DisasContext *s, arg_LD1_zprz *a, uint32_t insn)
>> +{
>> +    gen_helper_gvec_mem_scatter *fn = NULL;
>> +
>> +    if (a->esz < a->msz
>> +        || (a->msz == 0 && a->scale)
> Doesn't this check duplicate work you're already doing in the decode
> entries in the .decode file? If we're going to throw out msz==0 scale==1
> here we don't need to go to the effort to split this into 4 patterns
> so we can force the scale bit to 0 in the msz=00 line:
> 
>> +LD1_zprz        1100010 00 .0 ..... 0.. ... ..... ..... \
>> +                @rprr_g_load_xs_u esz=3 msz=0 scale=0
>> +LD1_zprz        1100010 01 .. ..... 0.. ... ..... ..... \
>> +                @rprr_g_load_xs_u_sc esz=3 msz=1
>> +LD1_zprz        1100010 10 .. ..... 0.. ... ..... ..... \
>> +                @rprr_g_load_xs_u_sc esz=3 msz=2
>> +LD1_zprz        1100010 11 .. ..... 0.. ... ..... ..... \
>> +                @rprr_g_load_xs_u_sc esz=3 msz=3
> and could just have
> LD1_zprz     1100010 msz:2 .. ..... 0.. ... ..... ..... \
>                 @rprr_g_load_xs_u_sc esz=3 scale=0
> 
> couldn't we?
> 
> (I don't really care which way round we do it.)
> 

I believe that I originally had the combined form, but it conflicts with some
other pattern (looking through the patterns, perhaps PRF?).  I must have
squashed the patch to split the pattern without removing the check in the helper.

> Is LDFF1 support going to appear later in this patchset?
> 
> For the moment should we UNDEF it?

Next patch, as you found.  Maybe I'll just drop the comment from this patch.  I
may also re-format the tables here, so they don't require so much re-formatting
in the next patch.


r~