[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);
----------------
benshi001 wrote:
> 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.
One more issue of doing 

```
Reserved.set(getTempReg());
Reserved.set(getZeroReg());
```

is that, we also have to reserve 16-bit registers `R0R1`/`R16R15`/`R17R16`/`R18/R17` on avrtiny. It makes sence to define `getTempReg()` and `getZeroRegs()`, but not for something like `getTinyReservedRegs()`.


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

https://reviews.llvm.org/D121672