[PATCH] [NVPTX] Move NVPTXPeephole after NVPTXPrologEpilogPass

Jingyue Wu jingyue at google.com
Tue Jun 30 22:23:03 PDT 2015


LGTM with the comments addressed.

The commit message could be clearer about what the bug was.

> NVPTXPrologEpilogPass relies on frame indices to compute frame offsets. NVPTXPeephole, if run before NVPTXPrologEpilogPass, would lose frame indices too early. Therefore, we move ...



================
Comment at: lib/Target/NVPTX/NVPTXPeephole.cpp:25
@@ -24,3 +24,3 @@
 // It will transform the following pattern
 //    %vreg0<def> = LEA_ADDRi64 <fi#0>, 4
 //    %vreg1<def> = cvta_to_local_yes_64 %vreg0
----------------
The comment here needs to be updated. 

================
Comment at: lib/Target/NVPTX/NVPTXPeephole.cpp:113
@@ -113,5 +112,3 @@
   // Get the correct offset
-  int FrameIndex = Prev.getOperand(1).getIndex();
-  int Offset = MF.getFrameInfo()->getObjectOffset(FrameIndex) +
-               Prev.getOperand(2).getImm();
+  auto Offset = Prev.getOperand(2).getImm();
 
----------------
`isCVTAToLocalCombination` should check Prev.getOperand(2) is an immediate; otherwise, things may break.  

================
Comment at: lib/Target/NVPTX/NVPTXPeephole.cpp:146-151
@@ +145,8 @@
+
+  const auto &MRI = MF.getRegInfo();
+  if (MRI.use_empty(NVPTX::VRFrame)) {
+    if (auto MI = MRI.getUniqueVRegDef(NVPTX::VRFrame)) {
+      MI->eraseFromParentAndMarkDBGValuesForRemoval();
+    }
+  }
+
----------------
Add a sentence explaining what this do. 

================
Comment at: lib/Target/NVPTX/NVPTXTargetMachine.cpp:213
@@ -214,2 +212,3 @@
   addPass(createNVPTXPrologEpilogPass(), false);
+  addPass(createNVPTXPeephole());
 }
----------------
Although you mentioned why they are in this order in your commit message, you also need to comment here because people won't usually dig into commit messages.

http://reviews.llvm.org/D10853

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list