[PATCH] D28300: [InstCombine] Fix address space handling when removing allocas

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:448
+
+    std::vector<Value *> Users(OldPtr->users().begin(), OldPtr->users().end());
+    for (auto U = Users.begin(); U != Users.end(); U++) {
----------------
SmallVector


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:472
+        // Create and insert new getelementptr instruction
+        std::vector<Value *> Indices(GEP->idx_begin(), GEP->idx_end());
+        auto NewGEP = GetElementPtrInst::Create(NULL, NewPtr, Indices);
----------------
SmallVector


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:473
+        std::vector<Value *> Indices(GEP->idx_begin(), GEP->idx_end());
+        auto NewGEP = GetElementPtrInst::Create(NULL, NewPtr, Indices);
+        NewGEP->insertAfter(GEP);
----------------
nullptr


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:478-481
+        auto NewLoad = new LoadInst(NewPtr);
+        NewLoad->setAlignment(LI->getAlignment());
+        NewLoad->setVolatile(LI->isVolatile());
+        NewLoad->insertAfter(LI);
----------------
You should be able to do all of this in the initial constructor


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:488-489
+        auto MIType = MI->getFunctionType();
+        std::vector<Type *> Types = {MIType->getParamType(0), NewPtr->getType(),
+                                     MIType->getParamType(2)};
+        auto NewMIFunc = Intrinsic::getDeclaration(MI->getModule(),
----------------
This can be a C array


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:505-507
+      } else {
+        assert(false && "Can't update address space for this instruction");
+      }
----------------
I think you'll also need to handle AtomicRMW and cmpxchg


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:62
 
+  SmallVector<unsigned, 35> ArgAddrSpaces;
+
----------------
35 is a strange choice for vector size


https://reviews.llvm.org/D28300