Re: [PATCH 10/33] target/mips: Extract df_extract() helper

[Richard Henderson]

On 10/23/21 2:47 PM, Philippe Mathieu-Daudé wrote:
> Extract the common code which parses data formats to an helper.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   target/mips/tcg/msa_translate.c | 68 +++++++++++++++++++--------------
>   1 file changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 3b0dfcca69d..7c1bbfaec61 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -272,6 +272,40 @@ static const char msaregnames[][6] = {
>       "w30.d0", "w30.d1", "w31.d0", "w31.d1",
>   };
>   
> +/* Encoding of Operation Field */
> +static const struct dfe {
> +    enum CPUMIPSMSADataFormat df;
> +    int start;
> +    int length;
> +    uint32_t value;
> +} df_elm[] = {
> +    /* Table 3.26 ELM Instruction Format */
> +    {DF_BYTE,   4, 2, 0b00},
> +    {DF_HALF,   3, 3, 0b100},
> +    {DF_WORD,   2, 4, 0b1100},
> +    {DF_DOUBLE, 1, 5, 0b11100}
> +}, df_bit[] = {
> +    /* Table 3.28 BIT Instruction Format */
> +    {DF_BYTE,   3, 4, 0b1110},
> +    {DF_HALF,   4, 3, 0b110},
> +    {DF_WORD,   5, 2, 0b10},
> +    {DF_DOUBLE, 6, 1, 0b0}
> +};

Possibly clearer as separate declarations instead of the comma.
The df field is redundant with the index.  At which point perhaps a

typedef const struct dfe dfe4[4];

might be helpful, both for the two declarations and...

> +/* Extract Operation Field (used by ELM & BIT instructions) */
> +static bool df_extract(const struct dfe *s, int value,
> +                       enum CPUMIPSMSADataFormat *df, uint32_t *x)

... the parameter.

> +    uint32_t df, m;
...
> +    if (!df_extract(df_bit, dfm, &df, &m)) {

How does the type of df not match the parameter?
Incomplete rebase?


r~