Re: [Qemu-devel] [RFC v3 19/24] riscv: tcg-target: Add the out op decoder

[Alistair Francis]

On Tue, Dec 11, 2018 at 2:45 PM Richard Henderson
<address@hidden> wrote:
>
> On 12/10/18 11:56 AM, Richard Henderson wrote:
> > On 12/7/18 6:49 PM, Alistair Francis wrote:
> >> +    case INDEX_op_neg_i64:
> >> +        tcg_out_opc_imm(s, OPC_SUB, a0, TCG_REG_ZERO, a1);
> >
> > tcg_out_opc_reg.
> >
> >> +    case INDEX_op_mulsh_i32:
> >> +    case INDEX_op_mulsh_i64:
> >> +        tcg_out_opc_imm(s, OPC_MULH, a0, a1, a2);
> >> +        break;
> >> +
> >> +    case INDEX_op_muluh_i32:
> >> +    case INDEX_op_muluh_i64:
> >> +        tcg_out_opc_imm(s, OPC_MULHU, a0, a1, a2);
> >> +        break;
> >
> > Likewise.
>
> Incidentally, catching these sorts of errors is why tcg/s390 puts the format 
> of
> the insn into the enum.  E.g.
>
> /* Emit an opcode with "type-checking" of the format.  */
> #define tcg_out_insn(S, FMT, OP, ...) \
>     glue(tcg_out_insn_,FMT)(S, glue(glue(FMT,_),OP), ## __VA_ARGS__)
>
>     tcg_out_insn(s, RR, AR, a0, a2);
>     tcg_out_insn(s, RRE, AGR, TCG_TMP0, index);
>
> /* All of the following instructions are prefixed with their instruction
>    format...  */
> typedef enum S390Opcode {
>     ...
>     RR_AR       = 0x1a,
>     RRE_AGR     = 0xb908,

That is really cool.

>
>
> I do something similar for tcg/aarch64, although that's more complicated due 
> to
> the wide variety of formats.
>
> Whether you go back and retro-fit this scheme to your existing patch set is up
> to you.  But I think it could be worthwhile.

It looks like I'm ready to send a patch series (instead of an RFC) so
I might leave it for now. There are a range of improvements we can
work on in the future, this can be yet another one.

Alistair

>
>
> r~