[PATCH] D40420: [CodeGen] Print "%vreg0" as "%0" in both MIR and debug output

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 27 11:16:46 PST 2017


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

LGTM with a bunch of nitpicks. Thanks!



================
Comment at: lib/CodeGen/DetectDeadLanes.cpp:24
+///    %3 = EXTRACT_SUBREG %2, sub1
+///           = use %3
+/// The %0 definition is dead and %3 contains an undefined value.
----------------
`= use` should move to the left


================
Comment at: lib/CodeGen/PeepholeOptimizer.cpp:1699
     //
-    // %PHYSREG is the map index; MI is the last valid `%vreg = COPY %PHYSREG`
+    // %PHYSREG is the map index; MI is the last valid `% = COPY %PHYSREG`
     // without any intervening re-definition of %PHYSREG.
----------------
maybe use `%[0-9]+` instead of just `%` in the example?


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1823-1824
     // Physreg is copied into vreg
-    //   %vregY = COPY %X
+    //   %Y = COPY %X
     //   ...  //< no other def of %X here
+    //   use %Y
----------------
rename `%X` to `%physreg_X` to make it clear it is not a vreg as `%Y`.


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1832-1834
+    //   %X = def
     //   ... //< no other def or use of %Y here
+    //   %Y = COPY %X
----------------
rename `%Y` to `%physreg_Y` to avoid confusion.


================
Comment at: lib/CodeGen/SplitKit.cpp:1378-1380
+      //   %827:subreg_hireg<def,read-undef> = ...
       //   ...
+      //   %828<def> = COPY %827
----------------
As we are touching this anyway: Rename to `%0` and `%1` as there is no reason for this to be 827/828 I believe.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:2833
   //
-  //   %vreg0<def> = COPY %XZR; GPR64common:%vreg0
+  //   %0<def> = COPY %XZR; GPR64common:%0
   //
----------------
(unrelated note: Wasn't this renamed to `%xzr` in your other commit?)


================
Comment at: lib/Target/BPF/BPFISelDAGToDAG.cpp:549
   unsigned AndOpReg = RegN->getReg();
-  DEBUG(dbgs() << "Examine %vreg" << TargetRegisterInfo::virtReg2Index(AndOpReg)
+  DEBUG(dbgs() << "Examine %" << TargetRegisterInfo::virtReg2Index(AndOpReg)
                << '\n');
----------------
Use `printReg()`?


================
Comment at: lib/Target/Hexagon/HexagonPeephole.cpp:16
 //    Into
-//    %vreg176<def> = COPY vreg166
+//    %176<def> = COPY vreg166
 //
----------------
`%166`. (Maybe do a round of grep `vreg[0-9]` through the sourcecode to see if there are other places where people missed the `%` in front of vreg.


https://reviews.llvm.org/D40420





More information about the llvm-commits mailing list