[PATCH] D132658: [InstCombine] Distributive or+mul with const operand

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:234
       // C1*MulC simplifies to a tidier constant.
       Value *NewC = Builder.CreateMul(C1, MulC);
       auto *BOp0 = cast<BinaryOperator>(Op0);
----------------
Allen wrote:
> spatel wrote:
> > Is there a reason to not use the more direct ConstExpr::getMul() API and make it explicit that NewC is a Constant *?
> Thanks your suggestion, done
@spatel, based on nikic's RFC, maybe we need avoid use ConstExpr.
https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:240
       auto *BO = BinaryOperator::CreateAdd(NewMul, NewC);
       auto *NewMulBO = dyn_cast<BinaryOperator>(NewMul);
+      if (I.hasNoUnsignedWrap() && Op0NUW && NewMulBO) {
----------------
If NewMulBO is constant we also can set BO to nuw. So the logic should be this
```
      if (I.hasNoUnsignedWrap() && Op0NUW && NewMulBO) {
        if (auto *NewMulBO = dyn_cast<BinaryOperator>(NewMul))
          NewMulBO->setHasNoUnsignedWrap();
        BO->setHasNoUnsignedWrap();
      }
```


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

https://reviews.llvm.org/D132658