[PATCH] D121672: [AVR] Reject/Reserve R0~R15 on AVRTiny

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRRegisterInfo.cpp:60-62
   Reserved.set(AVR::R0);
   Reserved.set(AVR::R1);
   Reserved.set(AVR::R1R0);
----------------
aykevl wrote:
> aykevl wrote:
> > I think it would make sense to change these hardcoded `R0`/`R1`/`R1R0` registers to properties of `AVRSubtarget` (a bit like `getIORegisterOffset` for example). This would make the code below a bit more sensible: it could just reserve `R0`-`R15` instead of `R2`-`R17` which looks a bit strange.
> > Such a change should probably need to be done in a separate patch, as a cleanup before this patch. Such a patch could also fix the hardcoded `R0`/`R1` registers in AVRFrameLowering.cpp, AVRExpandPseudoInsts.cpp, etc.
> It looks like you already added these getters to `AVRSubtarget` in D119807.
Thanks for your suggestion.

But I do not think reserve `R0`-`R15` looks good. Since `R0`&`R1` are always reserved on both avr and avrtiny. While `R2`-`R15` are only reserved on avrtiny. It looks strange like

```
if (STI.hasTinyEncoding()) {
    for (unsigned Reg = AVR::R0; Reg <= AVR::R17; Reg++)
      Reserved.set(Reg);
else {
      Reserved.set(R0);
      Reserved.set(R1);
}
```

My points are
1. `R0` and `R1` are always reserved, no matter avr or  avrtiny.
2. `R2`-`R17` are reserved only on avrtiny, while are available on avr.


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

https://reviews.llvm.org/D121672