[PATCH] D27283: Fix invalid addrspacecast due to combining alloca with global var

[David Majnemer via Phabricator via llvm-commits]

majnemer added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:238-240
+  auto *PT = dyn_cast<PointerType>(I.getType());
+  auto *NT = dyn_cast<PointerType>(V->getType());
+  assert(PT && NT && PT != NT && PT->getElementType() == NT->getElementType() &&
----------------
I'd make these cast<> instead of dyn_cast<> and remove PT and NT from the assert.


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:253-256
+      auto *NewI = GetElementPtrInst::Create(NT->getElementType(), V, Indices);
+      InsertNewInstWith(NewI, *GEP);
+      if (!replacePointer(*GEP, NewI))
+        return false;
----------------
Is it possible for you to create instructions which you never RAUW?


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:345-351
+        auto *SrcTy = TheSrc->getType();
+        auto *DestTy = PointerType::get(AI.getType()->getPointerElementType(),
+          SrcTy->getPointerAddressSpace());
         Constant *Cast
-          = ConstantExpr::getPointerBitCastOrAddrSpaceCast(TheSrc, AI.getType());
-        Instruction *NewI = replaceInstUsesWith(AI, Cast);
-        eraseInstFromFunction(*Copy);
-        ++NumGlobalCopies;
-        return NewI;
+          = ConstantExpr::getPointerBitCastOrAddrSpaceCast(TheSrc, DestTy);
+        if (AI.getType()->getPointerAddressSpace()
+            == SrcTy->getPointerAddressSpace()) {
----------------
Formatting looks funny.


https://reviews.llvm.org/D27283