[PATCH] D154791: [InstCombine] Transform bitwise (A >> C - 1, zext(icmp)) -> zext (bitwise(A < 0, icmp)) fold.

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1726
+                          m_SpecificInt(Op0->getType()->getScalarSizeInBits() - 1))) &&
+        match(Op1, m_ZExt(m_ICmp(Pred, m_Value(), m_Value()))) &&
+        LogicOpc != Instruction::Xor;
----------------
XChy wrote:
> goldstein.w.n wrote:
> > Does `Op1` need to be an `icmp`, or can it just be any `i1`?
> When folding `A >> BW - 1  -> A < 0`, there are many possible folds for `(A < 0) bitwise (icmp)`.
> However, if replacing `icmp` with an arbitrary i1, it seldom folds and just produces this single fold `A >> BW - 1  -> A < 0`, which is inefficient.
I see.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1727
+        match(Op1, m_ZExt(m_ICmp(Pred, m_Value(), m_Value()))) &&
+        LogicOpc != Instruction::Xor;
+
----------------
goldstein.w.n wrote:
> nit:: LogicOp != Instruction::Xor should go before the `match(...)` imo.
It seems in addressing this you just remove the LogicOp != Xor ingeneral...


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

https://reviews.llvm.org/D154791