[PATCH] D153698: [InstCombine] canonicalize multi xor as cmp+select

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4256
   // We cannot replace a constant, and shouldn't even try.
-  if (isa<Constant>(Op))
-    return nullptr;
-
-  auto *I = dyn_cast<Instruction>(V);
-  if (!I || !is_contained(I->operands(), Op))
+  if (isa<Constant>(Op) || !isa<Instruction>(V))
     return nullptr;
----------------
As we're now doing recursive calls, you need to guard against `MaxRecurse == 0` here.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4279
+    // TODO: support more type operator
+    if (!isa<BinaryOperator>(V))
+      return V;
----------------
These checks for BinaryOperator should not be necessary.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4292
+    return V;
+  });
 
----------------
You need to track whether any replacement happened. If it did not happen you can return early.


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

https://reviews.llvm.org/D153698