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

Daan Sprenkels via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 2 03:39:07 PDT 2019


dsprenkels marked an inline comment as done.
dsprenkels added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRInstrInfo.td:1682
+                        "rolb\t$rd",
+                        [(set i8:$rd, (AVRrol i8:$src)), (implicit SREG)]>;
+
----------------
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.


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

https://reviews.llvm.org/D60365





More information about the llvm-commits mailing list