On 5/30/19 2:07 PM, Michael Rolnik wrote: > This includes: > - encoding of all 16 bit instructions > - encoding of all 32 bit instructions > > Signed-off-by: Michael Rolnik <mrol...@gmail.com> > --- > target/avr/insn16.decode | 160 +++++++++++++++++++++++++++++++++++++++ > target/avr/insn32.decode | 10 +++ > 2 files changed, 170 insertions(+) > create mode 100644 target/avr/insn16.decode > create mode 100644 target/avr/insn32.decode
Two things: (1) decodetree can handle variable-width ISA now. It's slightly ugly in that the %field numbering is little-endian and thus varies for each insn size. But the in-flight patch set for target/rx shows that it works. That said, I don't think you need that because, (2) The four instructions that are 32-bits do not have any opcode bits in the second 16-bits. E.g. # The 22-bit immediate is partially in the opcode word, # and partially in the next. Use append_16 to build the # complete 22-bit value. %imm_call 4:5 0:1 !function=append_16 CALL 1001 010. .... 111. imm=%imm_call JMP 1001 010. .... 110. imm=%imm_call # The 16-bit immediate is completely in the next word. # Fields cannot be defined with no bits, so we cannot play # the same trick and append to a zero-bit value. # Defer reading the immediate until trans_{LDS,STS}. @ldst_s .... ... rd:5 .... imm=0 LDS 1001 000 ..... 0000 @ldst_s STS 1001 001 ..... 0000 @ldst_s static uint16_t next_word(DisasContext *ctx) { return cpu_lduw_code(ctx->env, ctx->npc++ * 2); } static int append_16(DisasContext *ctx, int x) { return x << 16 | next_word(ctx); } static bool trans_LDS(DisasContext *ctx, arg_ldst_s *a) { a->imm = next_word(ctx); // other stuff } I realize that next_word as written does not fit in to how you currently process instructions in the loop, but I also think that's a mistake. I'll respond to that in its place in the next patch. That said, next_word *could* be written to use ctx->inst[0].opcode. r~