[PATCH] D60365: [AVR] Fix codegen for rotate instructions

[Daan Sprenkels via Phabricator via llvm-commits]

dsprenkels added a comment.

Thank you for the feedback. I will try to update the patch code this weekend.



================
Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1273
+  // Put 0 in zero register
+  ZeroReg = scavengeGPR8(MI);
+  buildMI(MBB, MBBI, OpLoad)
----------------
dylanmckay wrote:
> This variable is called `ZeroReg`, and reading the code my inference is that the `ZeroReg` contains the register number of the zero-register, `r1`, which is a register that, [according to the calling convention](https://gcc.gnu.org/wiki/avr-gcc), is always guaranteed to contain the value `0`.
> 
> I recommend either modifying the logic to use the dedicated zero register instead of a scavenged temporary, or renaming the variable from `ZeroReg` to something like `TempRegWithZero`.
When I wrote the patch I hadn't yet found that r1 is always zero.  I will update the assembly to:

    add r16, r16
    adc r16, r1



================
Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1319
+
+  // Put 0 in temporary register
+  TmpReg = scavengeGPR8(MI);
----------------
dylanmckay wrote:
> Same here re. dedicated zero register.
Here, I'd like the keep the variable's name `TmpReg`, as its value is not exactly zero after executing the intstructions.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60365