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

[Allen zhong via Phabricator via llvm-commits]

Allen updated this revision to Diff 536983.
Allen added a comment.

address some comment
1、whether any replacement happened
2、 add condition to guard MaxRecurse == 0
3、delete the check for BinaryOperator, but add the following 3 type to avoid some regression

  **if (isa<SelectInst>(I) || isa<CmpInst>(I) || isa<LoadInst>(I))**


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

https://reviews.llvm.org/D153698

Files:
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/test/Transforms/InstCombine/select-cmp.ll


Index: llvm/test/Transforms/InstCombine/select-cmp.ll
===================================================================
--- llvm/test/Transforms/InstCombine/select-cmp.ll
+++ llvm/test/Transforms/InstCombine/select-cmp.ll
@@ -345,4 +345,18 @@
   ret i1 %r
 }
 
+; https://alive2.llvm.org/ce/z/TGgJTq
+define i32 @select_icmp_xor_multi_insn(i32 noundef %a, i32 noundef %b) {
+; CHECK-LABEL: @select_icmp_xor_multi_insn(
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[XOR1:%.*]] = xor i32 [[TMP1]], -1
+; CHECK-NEXT:    ret i32 [[XOR1]]
+;
+  %tobool = icmp eq i32 %a, %b
+  %not = xor i32 %a, -1
+  %xor1 = xor i32 %not, %b
+  %cond = select i1 %tobool, i32 -1, i32 %xor1
+  ret i32 %cond
+}
+
 declare void @use(i1)
Index: llvm/lib/Analysis/InstructionSimplify.cpp
===================================================================
--- llvm/lib/Analysis/InstructionSimplify.cpp
+++ llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4253,13 +4253,10 @@
     return RepOp;
 
   // 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) || MaxRecurse == 0)
     return nullptr;
 
+  auto *I = cast<Instruction>(V);
   // The arguments of a phi node might refer to a value from a previous
   // cycle iteration.
   if (isa<PHINode>(I))
@@ -4268,15 +4265,43 @@
   if (Op->getType()->isVectorTy()) {
     // For vector types, the simplification must hold per-lane, so forbid
     // potentially cross-lane operations like shufflevector.
-    assert(I->getType()->isVectorTy() && "Vector type mismatch");
+    assert((I->getType()->isVectorTy() || isa<LoadInst>(I)) &&
+           "Vector type mismatch");
     if (isa<ShuffleVectorInst>(I) || isa<CallBase>(I))
       return nullptr;
   }
 
   // Replace Op with RepOp in instruction operands.
   SmallVector<Value *, 8> NewOps(I->getNumOperands());
-  transform(I->operands(), NewOps.begin(),
-            [&](Value *V) { return V == Op ? RepOp : V; });
+  bool changed = false;
+  transform(I->operands(), NewOps.begin(), [&](Value *V) {
+    if (V == Op) {
+      changed |= true;
+      return RepOp;
+    }
+    if (!isa<Instruction>(V))
+      return V;
+    // Avoid some regression case.
+    if (isa<SelectInst>(I) || isa<CmpInst>(I) || isa<LoadInst>(I))
+      return V;
+    auto *NewI = cast<Instruction>(V);
+    if (NewI->getParent() != I->getParent())
+      return V;
+    // Implement only for a few non-refining
+    if (Value *S = simplifyWithOpReplaced(V, Op, RepOp, Q, 0, MaxRecurse - 1)) {
+      Constant *Cst = dyn_cast<Constant>(S);
+      // It is in fact no replacement when the return value equal to RepOp.
+      if (Cst && Cst != RepOp) {
+        changed |= true;
+        return S;
+      }
+    }
+    return V;
+  });
+
+  // Return early if it did not happen any replacement.
+  if (!changed)
+    return nullptr;
 
   if (!AllowRefinement) {
     // General InstSimplify functions may refine the result, e.g. by returning


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153698.536983.patch
Type: text/x-patch
Size: 3119 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230704/04dfbde9/attachment.bin>