[PATCH] D16209: Teach GlobalOpt not to drop DebugInfo on the floor when it promotes globals to allocas

[Jonathan Roelofs via llvm-commits]

jroelofs added inline comments.

================
Comment at: tools/llvm-project/llvm/lib/Transforms/IPO/GlobalOpt.cpp:1793
@@ -1779,2 +1792,3 @@
     if (!isa<UndefValue>(GV->getInitializer()))
-      new StoreInst(GV->getInitializer(), Alloca, &FirstI);
+      Store = new StoreInst(GV->getInitializer(), Alloca, &FirstI);
+
----------------
What happens in insertDelcare when this ^ is nullptr?

================
Comment at: tools/llvm-project/llvm/lib/Transforms/IPO/GlobalOpt.cpp:1796
@@ +1795,3 @@
+    if (DIGlobalVariable *D = FindDebugInfo(GV)) {
+      if (DIType *T = dyn_cast<DIType>(D->getRawType())) {
+        DEBUG(dbgs() << "                   ... and transferring its DebugInfo\n");
----------------
This ^ line in particular feels weird to me. I thought I should use D->getType().resolve(), but couldn't figure out how to get ahold of the parameter that it wants.

================
Comment at: tools/llvm-project/llvm/lib/Transforms/IPO/GlobalOpt.cpp:1798
@@ +1797,3 @@
+        DEBUG(dbgs() << "                   ... and transferring its DebugInfo\n");
+        auto *Scope = D->getScope();
+        auto *Variable = DIB->createAutoVariable(Scope,
----------------
Apparently this ^ isn't the scope that DILocation wants... not sure how to get the one it's looking for.

================
Comment at: tools/llvm-project/llvm/lib/Transforms/IPO/GlobalOpt.cpp:1806
@@ +1805,3 @@
+                                   Scope);
+        DIB->insertDeclare(Alloca, Variable, E, DL, Store);
+      }
----------------
Do I also need to insert the value one too?

================
Comment at: tools/llvm-project/llvm/lib/Transforms/IPO/GlobalOpt.cpp:3092
@@ -3064,2 +3091,3 @@
 bool GlobalOpt::runOnModule(Module &M) {
+  DIB = llvm::make_unique<DIBuilder>(M, /*AllowUnresolved=*/ false);
   bool Changed = false;
----------------
The `false` here is copy-pasta. I don't actually know if that's the value I want.


http://reviews.llvm.org/D16209