[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:1765
 
+/// Check if all users of CI are Store.
+static bool hasStoreUsersOnly(CastInst &CI) {
----------------
"Store" should probably be "StoreInsts".


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1767-1771
+  for (User *U : CI.users()) {
+    if (!isa<StoreInst>(U))
+      return false;
+  }
+  return true;
----------------
I think this can be simplified to `llvm::all_of(CI.users(), [](User *U) { return isa<StoreInst>(U); });`


================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1878
+      Worklist.Add(SI);
+      assert(hasStoreUsersOnly(*(CastInst *)NewBC));
+    }
----------------
This looks unidiomatic. I'd `cast<BitCastInst>` the result of `CreateBitCast`. `CreateBitCast` cannot be a `Constant` because its operand is a `PHINode`.


https://reviews.llvm.org/D23896