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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1709
+
+  auto GetOperandAsConstantInt = [&](Value *Op) -> ConstantInt * {
+    if (I.getType()->isVectorTy()) {
----------------
sdesmalen wrote:
> No variables really need to be captured for this function. You can also write it more compactly as:
> 
>   if (Op->getType()->isVectorTy())
>     if (auto *COp = dyn_cast<Constant>(Op))
>       return dyn_cast<ConstantInt>(COp->getSplatValue());
>   return dyn_cast<ConstantInt>(Op);
> 
> 
> No variables really need to be captured for this function. You can also write it more compactly as:
> 
>   if (Op->getType()->isVectorTy())
>     if (auto *COp = dyn_cast<Constant>(Op))
>       return dyn_cast<ConstantInt>(COp->getSplatValue());
>   return dyn_cast<ConstantInt>(Op);
> 
> 

That doesn't work. If vec is a non-splat constant it will fault from `dyn_cast<ConstantInt>(nullptr)`.

But it can be:
```
    if (Op->getType()->isVectorTy())
      if (auto *COp = dyn_cast<Constant>(Op)) {
        auto *CSplat = COp->getSplatValue();
        return CSplat ? dyn_cast<ConstantInt>(CSplat) : nullptr;
      }
    return dyn_cast<ConstantInt>(Op);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143014