[PATCH] D45976: [InstCombine] Simplify Add with remainder expressions as operands.

[Bixia Zheng via Phabricator via llvm-commits]

bixia added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1087
+                match(E, m_LShr(m_Value(Op), m_ConstantInt(CI))))) {
+      C = CI->getValue();
+      if (cast<Instruction>(E)->getOpcode() == Instruction::LShr) {
----------------
sanjoy wrote:
> Probably better to do this as a separate change, but it might be nice to push all of this logic (division as `udiv` or as an `lshr`) into a matcher.
Will consider doing this in another change.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1062
+        match(E, m_URem(m_Value(Op), m_APInt(CI))) ||
+        (match(E, m_And(m_Value(Op), m_APInt(CI))) && (*CI + 1).isPowerOf2())) {
+      C = *CI;
----------------
sanjoy wrote:
> Instead of `(*CI + 1).isPowerOf2()` you can do `CI->isAllOnesValue()` to avoid the addition and also read a bit cleaner.
the two are not equivalent.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1081
+      return true;
+    } else if (!IsSigned && (match(E, m_UDiv(m_Value(Op), m_APInt(CI))) ||
+                             match(E, m_LShr(m_Value(Op), m_APInt(CI))))) {
----------------
sanjoy wrote:
> Can this match a `ConstantExpr` (which isn't an `Instruction` and will fail the `cast<>` below)?  Or have we checked before that `E` is an `Instruction`?
Remove the use of the cast.


Repository:
  rL LLVM

https://reviews.llvm.org/D45976