Reapply [InstSimplify] Support all instructions in simplifyWithOpReplaced()
authorNikita Popov <npopov@redhat.com>
Mon, 24 Apr 2023 13:22:20 +0000 (15:22 +0200)
committerNikita Popov <npopov@redhat.com>
Tue, 25 Apr 2023 13:45:32 +0000 (15:45 +0200)
Relative to the previous attempt, this includes a bailout for phi
nodes, whose arguments might refer to a previous cycle iteration.

We did not hit this before by a fortunate deficiency of the
ConstantFoldInstOperands() API, which doesn't handle phi nodes,
unlike ConstantFoldInstruction().

-----

Instead of hardcoding a few instruction kinds, use the generic
interface now that we have it.

The primary effect of this is that intrinsics are now supported.

It's worth noting that this is still limited in that it does not
support vectors, so we can't remove e.g. existing fshl special
cases.

llvm/lib/Analysis/InstructionSimplify.cpp
llvm/test/Transforms/InstSimplify/select-maxmin.ll

index 0097c61..f45ad98 100644 (file)
@@ -74,6 +74,10 @@ static Value *simplifyGEPInst(Type *, Value *, ArrayRef<Value *>, bool,
                               const SimplifyQuery &, unsigned);
 static Value *simplifySelectInst(Value *, Value *, Value *,
                                  const SimplifyQuery &, unsigned);
+static Value *simplifyInstructionWithOperands(Instruction *I,
+                                              ArrayRef<Value *> NewOps,
+                                              const SimplifyQuery &SQ,
+                                              unsigned MaxRecurse);
 
 static Value *foldSelectWithBinaryOp(Value *Cond, Value *TrueVal,
                                      Value *FalseVal) {
@@ -4206,6 +4210,11 @@ static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
   if (!I || !is_contained(I->operands(), Op))
     return nullptr;
 
+  // The arguments of a phi node might refer to a value from a previous
+  // cycle iteration.
+  if (isa<PHINode>(I))
+    return nullptr;
+
   if (Op->getType()->isVectorTy()) {
     // For vector types, the simplification must hold per-lane, so forbid
     // potentially cross-lane operations like shufflevector.
@@ -4270,23 +4279,8 @@ static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
       return Simplified != V ? Simplified : nullptr;
     };
 
-    if (auto *B = dyn_cast<BinaryOperator>(I))
-      return PreventSelfSimplify(simplifyBinOp(B->getOpcode(), NewOps[0],
-                                               NewOps[1], Q, MaxRecurse - 1));
-
-    if (CmpInst *C = dyn_cast<CmpInst>(I))
-      return PreventSelfSimplify(simplifyCmpInst(C->getPredicate(), NewOps[0],
-                                                 NewOps[1], Q, MaxRecurse - 1));
-
-    if (auto *GEP = dyn_cast<GetElementPtrInst>(I))
-      return PreventSelfSimplify(simplifyGEPInst(
-          GEP->getSourceElementType(), NewOps[0], ArrayRef(NewOps).slice(1),
-          GEP->isInBounds(), Q, MaxRecurse - 1));
-
-    if (isa<SelectInst>(I))
-      return PreventSelfSimplify(simplifySelectInst(
-          NewOps[0], NewOps[1], NewOps[2], Q, MaxRecurse - 1));
-    // TODO: We could hand off more cases to instsimplify here.
+    return PreventSelfSimplify(
+        ::simplifyInstructionWithOperands(I, NewOps, Q, MaxRecurse - 1));
   }
 
   // If all operands are constant after substituting Op for RepOp then we can
index 34a3203..68d16c8 100644 (file)
@@ -2136,10 +2136,7 @@ define <4 x i8> @ult_yx_umax_select_y_shuf_mask_fval(<4 x i8> %x, <4 x i8> %y) {
 
 define i8 @select_umin_with_icmp_zero(i8 %x, i8 %y) {
 ; CHECK-LABEL: @select_umin_with_icmp_zero(
-; CHECK-NEXT:    [[MIN:%.*]] = call i8 @llvm.umin.i8(i8 [[X:%.*]], i8 [[Y:%.*]])
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8 [[X]], 0
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP]], i8 [[MIN]], i8 0
-; CHECK-NEXT:    ret i8 [[SEL]]
+; CHECK-NEXT:    ret i8 0
 ;
   %min = call i8 @llvm.umin.i8(i8 %x, i8 %y)
   %cmp = icmp eq i8 %x, 0