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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2043
+
+  while (OrOperator) {
+    Value *OrLhs = OrOperator->getOperand(0),
----------------
AFAICT this condition never comes into play. You check the loop condition before reseting `OrOperator` every time. I'd say either make this a `while(1)` or make this `BO && B0->getOpcode() == Instruction::Or`


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2066
+
+    if (!OrLhsIsXor && !OrRhsIsXor) {
+      break;
----------------
There could we a case where we have:

`(or (or (xor A, B), (xor C, D)), (or (xor E, F), (xor G, H)))`.
Not sure if it worth handling this case, but maybe switch this to a worklist algo so you can arbitrarily add `Or` ops.


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

https://reviews.llvm.org/D154306