Re: [PATCH v8 13/35] Hexagon (target/hexagon) instruction/packet decode

[Richard Henderson]

On 2/7/21 9:46 PM, Taylor Simpson wrote:
> +#define DECODE_MAPPED_REG(REGNO, NAME) \
> +    insn->regno[REGNO] = DECODE_REGISTER_##NAME[insn->regno[REGNO]];

The macro argument is not a regno, but an operand number.

> +static inline int decode_opcode_can_jump(int opcode)
> +{
> +    if ((GET_ATTRIB(opcode, A_JUMP)) ||
> +        (GET_ATTRIB(opcode, A_CALL)) ||
> +        (opcode == J2_trap0) ||
> +        (opcode == J2_pause)) {
> +        /* Exception to A_JUMP attribute */
> +        if (opcode == J4_hintjumpr) {
> +            return 0;
> +        }
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +static inline int decode_opcode_ends_loop(int opcode)
> +{
> +    return GET_ATTRIB(opcode, A_HWLOOP0_END) ||
> +           GET_ATTRIB(opcode, A_HWLOOP1_END);
> +}

You can drop the inline markers.
You should use bool for booleans.

> +/*
> + * Shuffle for execution
> + * Move stores to end (in same order as encoding)
> + * Move compares to beginning (for use by .new insns)
> + */
> +static void decode_shuffle_for_execution(Packet *packet)
> +{
> +    int changed = 0;
> +    int i;
> +    int flag;    /* flag means we've seen a non-memory instruction */

Both changed and flag should be bool.  Please have another look through and use
bool as reasonable.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~