[PATCH] D132412: [InstCombine] reassociate/factor add+mul with common operand

[Allen zhong via Phabricator via llvm-commits]

Allen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1423
+    Value *IncY = Builder.CreateAdd(Y, ConstantInt::get(I.getType(), 1));
+    return BinaryOperator::CreateMul(IncY, X);
+  }
----------------
should we need consider the flag of NUW and NSW ?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1422
+                        m_OneUse(m_c_Add(m_Deferred(X), m_Value(Z)))))) {
+    // (X * Y) + (X + Z) --> ((Y + 1) * X) + Z
+    Value *IncY = Builder.CreateAdd(Y, ConstantInt::get(I.getType(), 1));
----------------
bcl5980 wrote:
> hiraditya wrote:
> > Can this cause a regression when Y  is a power of two?
> > Some other optimization would rewrite `x*y` as `y << n (when n == lg(y) at compile time)`?
> https://alive2.llvm.org/ce/z/LRobTp
> It looks the answer is yes.
so, the decomposition the const of a MUL should be improved first?


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

https://reviews.llvm.org/D132412