[PATCH] D154306: [InstCombine] Generalise ((x1 ^ y1) | (x2 ^ y2)) == 0 transform to more than two pairs of variables

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2041
+  SmallVector<std::pair<Value *, Value *>, 2> CmpValues;
+  SmallVector<llvm::Value *, 16> WorkList(1, Or);
+
----------------
Unnecessary `llvm::` prefix here and elsewhere.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2052
+    Value *OrLhs = OrOperator->getOperand(0),
+          *OrRhs = OrOperator->getOperand(1);
+
----------------
goldstein.w.n wrote:
> Prefer not name these `Or*` as you expect to match them for `Xor`. Can you just make these `Lhs`/`Rhs`.
These are the Lhs/Rhs of an Or though. Lhs/Rhs of the Xor are matched below.

Not sure why this isn't doing `match(CurrentValue, m_Or(m_Value(OrLhs), m_Value(OrRhs))` though.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2080
+                                       CmpValues.begin()->second);
+    return BinaryOperator::Create(BOpc, LhsCmp, RhsCmp);
   }
----------------
Why does this one need to be handled separately?


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

https://reviews.llvm.org/D154306