[PATCH] Add NVPTXLowerAlloca pass to convert alloca'ed memory to local address

[Jingyue Wu]

================
Comment at: lib/Target/NVPTX/NVPTXLowerAlloca.cpp:10
@@ +9,3 @@
+//
+// For all alloca instructions, and add a cast to local address for
+// each of them. For example,
----------------
The comment here is imprecise. You're not adding "a cast to local"; you are adding a *pair* of addrspacecasts. 

================
Comment at: lib/Target/NVPTX/NVPTXLowerAlloca.cpp:25
@@ +24,3 @@
+// two instructions.
+//
+//===----------------------------------------------------------------------===//
----------------
I'd mention the end result -- what the PTX looks like after this change, because that's what you are optimizing for. 

================
Comment at: lib/Target/NVPTX/NVPTXLowerAlloca.cpp:65
@@ +64,3 @@
+bool NVPTXLowerAlloca::runOnFunction(Function &F) {
+  for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
+    for (BasicBlock::iterator BI = I->begin(), BE = I->end(); BI != BE;) {
----------------
Can this pass be a BasicBlockPass? Looks like all the changes are intra-BB. 

================
Comment at: lib/Target/NVPTX/NVPTXLowerAlloca.cpp:66
@@ +65,3 @@
+  for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
+    for (BasicBlock::iterator BI = I->begin(), BE = I->end(); BI != BE;) {
+      if (auto allocaInst = dyn_cast<AllocaInst>(BI++)) {
----------------
Prefer
```
for (auto &B : F) {
  for (auto &I : B) {
```
unless you invalidate the iterators. 

================
Comment at: lib/Target/NVPTX/NVPTXLowerAlloca.cpp:77
@@ +76,3 @@
+        NewASCToGeneric->insertAfter(NewASCToLocal);
+        for (Value::use_iterator UI = allocaInst->use_begin(),
+                                 UE = allocaInst->use_end();
----------------
Similarly, I think there's a `uses()` that returns a range? 

================
Comment at: lib/Target/NVPTX/NVPTXLowerAlloca.cpp:81
@@ +80,3 @@
+          // Check Load, Store, GEP Uses on alloca and make them use the
+          // converted generic address.
+          const auto &AllocaUse = *UI++;
----------------
and BitCast. Can you comment in code why you only check these types? What would happen if you replace all uses? 

================
Comment at: test/CodeGen/NVPTX/call-with-alloca-buffer.ll:52
@@ -51,3 +52,2 @@
 
-; CHECK: add.u64 %rd[[SP_REG:[0-9]+]], %SP, 0
 ; CHECK:        .param .b64 param0;
----------------
Is this `%SP + 0` removed due to your change? If so, add a `CHECK-NOT` in the test.

http://reviews.llvm.org/D10483

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