[PATCH] D128602: Apply code review suggestions

Patryk Wychowaniec via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 26 01:08:50 PDT 2022


Patryk27 created this revision.
Herald added subscribers: Jim, hiraditya, dylanmckay.
Herald added a project: All.
Patryk27 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128602

Files:
  llvm/lib/Target/AVR/AVRInstrInfo.cpp
  llvm/test/CodeGen/AVR/pseudo/COPY.mir


Index: llvm/test/CodeGen/AVR/pseudo/COPY.mir
===================================================================
--- llvm/test/CodeGen/AVR/pseudo/COPY.mir
+++ llvm/test/CodeGen/AVR/pseudo/COPY.mir
@@ -28,7 +28,7 @@
     ; CHECK-NEXT: mov r23, r25
 
     $r23r22 = COPY $r25r24
-    RCALLk @foo, implicit $sp, implicit killed $r24r23
+    RCALLk @foo, implicit $r24r23
 ...
 
 ---
@@ -43,5 +43,5 @@
     ; CHECK-NEXT: mov r24, r23
 
     $r25r24 = COPY $r24r23
-    RCALLk @foo, implicit $sp, implicit killed $r25r24
+    RCALLk @foo, implicit $r25r24
 ...
Index: llvm/lib/Target/AVR/AVRInstrInfo.cpp
===================================================================
--- llvm/lib/Target/AVR/AVRInstrInfo.cpp
+++ llvm/lib/Target/AVR/AVRInstrInfo.cpp
@@ -58,34 +58,7 @@
       TRI.splitReg(DestReg, DestLo, DestHi);
       TRI.splitReg(SrcReg, SrcLo, SrcHi);
 
-      bool CopyHasOverlappingRegs = DestLo == SrcHi;
-
-      // Usually, for readability & consistency, we emit:
-      //
-      // ```
-      // mov $DestLo, $SrcLo
-      // mov $DestHi, $SrcHi
-      // ```
-      //
-      // ... but if our registers overlap, then we have to switch the order:
-      //
-      // ```
-      // mov $DestHi, $SrcHi
-      // mov $DestLo, $SrcLo
-      // ```
-      //
-      // ... as not to garble the operation.
-      //
-      // E.g. imagine trying to expand `$r25r24 = COPY $r24r23` - doing it the
-      // first way would yield:
-      //
-      // ```
-      // mov r24, r23
-      // mov r25, r24
-      // ```
-      //
-      // ... which would always set both registers to `r23`.
-      if (CopyHasOverlappingRegs) {
+      if (DestLo == SrcHi) {
         BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestHi)
             .addReg(SrcHi, getKillRegState(KillSrc));
         BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestLo)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128602.440045.patch
Type: text/x-patch
Size: 1830 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220626/8a286954/attachment.bin>


More information about the llvm-commits mailing list