[AVR] Add AVR MOVW/ADIW/SUBIW disassembly

[tomtor]

Fix <unknown> in llvm-objdump -d output

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[tomtor]

@Patryk27 I hope you don't mind that I have a question again about the LLVM code base. I made a number of PRs which complete disassembly of AVR files with llvm_objdump. This one is the last, it adds MOVW and ADIW/SBIW (SUBIW). All other PRs decode fine, but this one leaves the ADIW/SBIW as:

      f4: 02 96         adiw    r24, <unknown>

so the opcode mnemonic and register are OK, but the immediate is not. Any hints?

[Patryk27]

Nice!

I think it throws <unknown> because internally this instruction takes three arguments - $rd, $src, and $k:

llvm-project/llvm/lib/Target/AVR/AVRInstrInfo.td

Line 398 in 0494f93

def ADIWRdK : FWRdK<0b0, (outs IWREGS:$rd), (ins IWREGS :$src, imm_arith6:$k),

Since this doesn't make sense from assembly's point of view (where $rd = $src), you just have to duplicate the register:

Inst.setOpcode((Insn & 0xff00) == 0x9600 ? AVR::ADIWRdK : AVR::SUBIWRdK);
Inst.addOperand(MCOperand::createReg(RegVal));
Inst.addOperand(MCOperand::createReg(RegVal));
unsigned imm = ((Insn & 0x00C0) >> 2) | (Insn & 0xF);
Inst.addOperand(MCOperand::createImm(imm));
return MCDisassembler::Success;

[tomtor]

@Patryk27 Aha, that makes sense. All is OK now. Thanks!

[tomtor]

@benshi001 Hi, can you please have a look? With this and the other PRs llvm-objdump -d should disassemble any AVR output without issues. If I have missed an instruction, please let me know.

[Patryk27]

I can add a comment up-front - is it possible to test this code? Not sure if we have some tests for the disassembler (away from the keyboard at the moment), but I'd expect a couple.

[benshi001]

I can add a comment up-front - is it possible to test this code? Not sure if we have some tests for the disassembler (away from the keyboard at the moment), but I'd expect a couple.

We have decoding tests, such as
https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/AVR/inst-adiw.s
https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/AVR/inst-movw.s

[benshi001]

@benshi001 Hi, can you please have a look? With this and the other PRs llvm-objdump -d should disassemble any AVR output without issues. If I have missed an instruction, please let me know.

Thanks for your patch, I really appreciate your investigation into this issue. But I do not think this is a good solution. Since llvm can disassemble ADIW/SBIW correctly with --mcpu=avr2 specified to llvm-objdump.

However there is still an underlying issue, which I have recorded as #146451 .

[benshi001]

Please refer to #146451 for my suggestion.

[benshi001]

Please refer to #146451 for my suggestion.

@Patryk27, what's your opinion on my solution ?

[tomtor]

The PRs I create are often triggered by issues I encounter when using llvm/rust. I used llvm-objdump on a test program output. Will investigate which instructions are missing with the objdump flag -mcpu

[tomtor]

@benshi001 -mcpu=avr128db28 looks to generate correct output for my test program

[tomtor]

I can add a comment up-front - is it possible to test this code? Not sure if we have some tests for the disassembler (away from the keyboard at the moment), but I'd expect a couple.

I did not find any existing tests.

[Patryk27]

@Patryk27, what's your opinion on my solution ?

Oh, I haven't played with the disassembler before, so I didn't even realize LLVM is supposed to (mostly) auto-disassemble instructions.

I think your suggestion is correct, i.e. the fundamental issue is that not all instructions are included in the base instruction set and LLVM will fail to "auto-disassemble" those which require to be opt-in (or so I think).

*.elf files can already encode different AVR architectures (EF_AVR_ARCH), so what we should probably do is teach llvm-objdump to read that field.

[tomtor]

@Patryk27 I also did not realize that llvm-objdump needs that flag, probably because AVR support is marginal in other places (-mrelax, newer device issues, etc).

Wonder if llvm-objdump also needs a flag for eg arm devices?