[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-348
+  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(TI))
+    return BinaryOperator::Create(BO->getOpcode(), Op0, Op1);
+  else if (GetElementPtrInst *TGEP = dyn_cast<GetElementPtrInst>(TI)) {
----------------
Please consistently brace.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:347-348
+  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(TI))
+    return BinaryOperator::Create(BO->getOpcode(), Op0, Op1);
+  else if (GetElementPtrInst *TGEP = dyn_cast<GetElementPtrInst>(TI)) {
+    GetElementPtrInst *FGEP = dyn_cast<GetElementPtrInst>(FI);
----------------
Don't else after a return: https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:351-353
+    return TGEP->isInBounds() && FGEP->isInBounds()
+      ? GetElementPtrInst::CreateInBounds(ElementType, Op0, { Op1 })
+      : GetElementPtrInst::Create(ElementType, Op0, { Op1 });
----------------
Is this clang-format'd?


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:354-355
+      : GetElementPtrInst::Create(ElementType, Op0, { Op1 });
+  } else
+    llvm_unreachable("Expected BinaryOperator or GEP");
+  return nullptr;
----------------
Brace this too.


Repository:
  rL LLVM

https://reviews.llvm.org/D39958