Re: [PATCH v2 21/32] target/mips: Convert MSA 3R instruction format to decodetree (part 2/4)

[Philippe Mathieu-Daudé]

On 10/28/21 00:02, Richard Henderson wrote:
> On 10/27/21 11:07 AM, Philippe Mathieu-Daudé wrote:
>> Convert 3-register operations to decodetree.
>>
>> Per the Encoding of Operation Field for 3R Instruction Format'
>> (Table 3.25), these instructions are not defined for the BYTE
>> format. Therefore the TRANS_DF_iii_b() macro returns 'false'
>> in that case, because no such instruction is decoded.
>>
>> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> v2: TRANS_DF_iii_b() uses array[4]
>> ---
>>   target/mips/tcg/msa.decode      |  11 ++
>>   target/mips/tcg/msa_translate.c | 195 ++++++--------------------------
>>   2 files changed, 48 insertions(+), 158 deletions(-)

>>   +#define TRANS_DF_iii_b(NAME, trans_func, gen_func) \
>> +    static gen_helper_piii * const NAME##_tab[4] = { \
>> +        gen_func##_h, gen_func##_w, gen_func##_d \
>> +    }; \
>> +    static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
>> +    { \
>> +        if (a->df == DF_BYTE) { \
>> +            return false; \
>> +        } \
>> +        if (!check_msa_enabled(ctx)) { \
>> +            return true; \
>> +        } \
>> +        return trans_func(ctx, a, NAME##_tab[a->df - DF_HALF]); \
> 
> Either reduce the size of the array by one, or place the NULL in its
> proper place at the beginning rather than the end of the array.  I think
> the latter is in the end clearer.

Yes.

> You could just as well place the checks within trans_msa_3r:
> 
>     if (gen_msa_3r == NULL) {
>         return false;
>     }
>     if (!check_msa_enabled(ctx)) {
>         return true;
>     }

Indeed, thank you.