[PATCH] D39958: [InstCombine] Make foldSelectOpOp able to handle two-operand getelementptr

[David Majnemer via Phabricator via llvm-commits]

majnemer added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:346
   Value *Op1 = MatchIsOpZero ? NewSI : MatchOp;
-  return BinaryOperator::Create(BO->getOpcode(), Op0, Op1);
+  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(TI)) {
+    return BinaryOperator::Create(BO->getOpcode(), Op0, Op1);
----------------
auto *BO


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:349
+  }
+  if (GetElementPtrInst *TGEP = dyn_cast<GetElementPtrInst>(TI)) {
+    GetElementPtrInst *FGEP = dyn_cast<GetElementPtrInst>(FI);
----------------
I think this dyn_cast can just be cast<>, then you can avoid the llvm_unreachable.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:350
+  if (GetElementPtrInst *TGEP = dyn_cast<GetElementPtrInst>(TI)) {
+    GetElementPtrInst *FGEP = dyn_cast<GetElementPtrInst>(FI);
+    Type *ElementType = TGEP->getResultElementType();
----------------
You dyn_cast but unconditionally deference. Should this be cast<> ?


Repository:
  rL LLVM

https://reviews.llvm.org/D39958