Richard,
I don't understand what I should do. Do you want me to merge decode files?
On Fri, May 31, 2019 at 5:45 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> 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~
>
--
Best Regards,
Michael Rolnik
