[PATCH] D23896: [InstCombine] Try to resubmit the combine of A->B->A BitCast and fix for pr27996

[David Majnemer via llvm-commits]

majnemer added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1873
+      if (auto *C = dyn_cast<Constant>(V)) {
+        NewV = Builder->CreateBitCast(C, DestTy);
+      } else if (auto *LI = dyn_cast<LoadInst>(V)) {
----------------
It'd be more obvious why you don't need to mess with the builder insertion point if you used `ConstantExpr::getBitCast` instead.


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1875-1876
+      } else if (auto *LI = dyn_cast<LoadInst>(V)) {
+        Builder->SetInsertPoint(OldPN->getIncomingBlock(j)->getTerminator());
+        NewV = Builder->CreateBitCast(LI, DestTy);
+        Worklist.Add(LI);
----------------
I think this would do the wrong thing if the incoming block was an EH pad.  Could we insert the bitcast after `LI`?


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1893-1894
+      Builder->SetInsertPoint(SI);
+      BitCastInst *NewBC = cast<BitCastInst>(
+                           Builder->CreateBitCast(NewPNodes[PN], SrcTy));
+      SI->setOperand(0, NewBC);
----------------
`auto *`, also please clang-format this line.


https://reviews.llvm.org/D23896