[PATCH] D96394: [AVR] Improve inline assembly

[Ben Shi via Phabricator via llvm-commits]

benshi001 added a comment.

In D96394#2602957 <https://reviews.llvm.org/D96394#2602957>, @aykevl wrote:

> I don't really understand what you're doing here. For example:
>
>   case 'd': // Upper registers r16..r31.
>     if (VT == MVT::i8)
>       return std::make_pair(0U, &AVR::LD8RegClass);
>     else if (VT == MVT::i16)
>       return std::make_pair(0U, &AVR::DLDREGSRegClass);
>     break;
>
> If I'm reading https://www.nongnu.org/avr-libc/user-manual/inline_asm.html correctly, this appears to imply an 8-bit register, but you're also implementing support for 16-bit registers. Why?

Although I expect 'd' constraint to imply an 8-bit variable, I can not prevent user specifies a 16-bit variable as `d` constraint on purpose. If `AVR::LD8RegClass` is returned for a 16-bit variable with `d`, llvm will crash as reported in https://bugs.llvm.org/show_bug.cgi?id=48480 .

The logic here is,

1. return `AVR::LD8RegClass` for 8-bit `d`, which is correct and expected.
2. return `AVR::DLDREGSRegClass` for 16-bit `d`, which is not expected, but also correct assembly.
3. `break` to llvm's default process for 32/64-bit `d`, which is not expected, but also wrong assembly.

Actually avr-gcc accepts all 8-16-32-64-bit `d` and generates all correct assembly. And I just fix 8-16 bit in the patch, and left 32-64-bit in furture patches.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96394/new/

https://reviews.llvm.org/D96394