[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