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

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay accepted this revision.
dylanmckay added a comment.
This revision is now accepted and ready to land.

Approved, sorry for the super long turn around on this @dsprenkels, I wanted to get an simulated integration test passing before I landed this and I finally worked out all the kinks.

Nice patch, thanks for the hard work!



================
Comment at: llvm/lib/Target/AVR/AVRInstrInfo.td:1682
+                        "rolb\t$rd",
+                        [(set i8:$rd, (AVRrol i8:$src)), (implicit SREG)]>;
+
----------------
dsprenkels wrote:
> dsprenkels wrote:
> > dylanmckay wrote:
> > > What does the `b` stand for in the instruction mnemonics?
> > > 
> > > Perhaps it would make sense to rename the AVR ISD nodes (such as `AVRrol` and `AVRror` to `AVRbrol` `AVRbror`).
> > In this mnemonic, `b` stands for byte. Line 1689 defines the `ROLWRd` pseudo-instruction, so it makes sense to call this one `ROLBRd`.
> > 
> > > Perhaps it would make sense to rename the AVR ISD nodes (such as AVRrol and AVRror to AVRbrol AVRbror).
> > I totally agree.
> >>   Perhaps it would make sense to rename the AVR ISD nodes (such as AVRrol and AVRror to AVRbrol AVRbror).
> > I totally agree.
> 
> I refrained from doing this. I am not really sure which instructions should get the `b` suffix. If we add the suffix to the rotate instructions, should we also add them to shift instructions, etc. I hope we could postpone this improvement to another patch.
Sounds good to me @dsprenkels 


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

https://reviews.llvm.org/D60365