[PATCH] D143014: [InstCombine] Add combines for `(urem/srem (mul/shl X, Y), (mul/shl X, Z))`

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1707-1710
+  // BO0 = X * Y
+  BinaryOperator *BO0 = dyn_cast<BinaryOperator>(Op0);
+  // BO1 = X * Z
+  BinaryOperator *BO1 = dyn_cast<BinaryOperator>(Op1);
----------------
These can be `cast<BinaryOperator>` instead, since we know (from the `match` above) that they're either Mul or Shl, so they must be BinaryOperator's.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1720-1733
+  auto ConstY = dyn_cast<ConstantInt>(Y);
+  auto ConstZ = dyn_cast<ConstantInt>(Z);
+  if (I.getType()->isVectorTy()) {
+    auto VConstY = dyn_cast<Constant>(Y);
+    auto VConstZ = dyn_cast<Constant>(Z);
+    if (VConstY && VConstZ) {
+      VConstY = VConstY->getSplatValue();
----------------
nit: You can write this a bit shorter like this:

  ConstantInt *ConstY = nullptr, *ConstZ = nullptr;
  if (I.getType()->isVectorTy()) {
    if (auto *CY = dyn_cast<Constant>(Y))
      ConstY = dyn_cast<ConstantInt>(CY->getSplatValue());
    if (auto *CZ = dyn_cast<Constant>(Z))
      ConstZ = dyn_cast<ConstantInt>(CZ->getSplatValue());
  } else {
    ConstY = dyn_cast<ConstantInt>(Y);
    ConstZ = dyn_cast<ConstantInt>(Z);
  } 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143014/new/

https://reviews.llvm.org/D143014