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

[Allen zhong via Phabricator via llvm-commits]

Allen marked 6 inline comments as done.
Allen added a comment.

In D132658#3754136 <https://reviews.llvm.org/D132658#3754136>, @spatel wrote:

> Please pre-commit the baseline tests. If something is not transforming, you can add a comment in this patch to make it clear that it is a negative test or TODO item.

Addressed in the link D132820 <https://reviews.llvm.org/D132820>, and I'll rebase this commit after D132820 <https://reviews.llvm.org/D132820> merged, thanks!



================
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);
----------------
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


================
Comment at: llvm/test/Transforms/InstCombine/mul.ll:746
+;
+  %shl = shl nuw <2 x i32> %v1, <i32 2, i32 undef>
+  %add = or <2 x i32> %shl, <i32 3, i32 undef>
----------------
spatel wrote:
> Replace "undef" with "poison" in this test - we are transitioning away from undef in IR.
Thanks, apply with your comment


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

https://reviews.llvm.org/D132658