Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ConstraintElim] Simplify MinMaxIntrinsic #75306

Merged
merged 4 commits into from
Feb 4, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 13, 2023

This patch replaces min/max intrinsic with one of its operands if possible.
Alive2: https://alive2.llvm.org/ce/z/LoHfYf
Fixes #75155.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 13, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch replaces min/max intrinsic with one of its operands if possible.
Fixes #75155.


Full diff: https://github.com/llvm/llvm-project/pull/75306.diff

4 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+65-12)
  • (modified) llvm/test/Transforms/ConstraintElimination/debug.ll (+3-3)
  • (modified) llvm/test/Transforms/ConstraintElimination/minmax.ll (+141)
  • (modified) llvm/test/Transforms/ConstraintElimination/reproducer-remarks-debug.ll (+1-1)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index fafbe17583f5da..6a991f694476c8 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1009,6 +1009,13 @@ void State::addInfoFor(BasicBlock &BB) {
 
     if (isa<MinMaxIntrinsic>(&I)) {
       WorkList.push_back(FactOrCheck::getInstFact(DT.getNode(&BB), &I));
+      for (Use &U : I.uses()) {
+        auto *UserI = getContextInstForUse(U);
+        auto *DTN = DT.getNode(UserI->getParent());
+        if (!DTN)
+          continue;
+        WorkList.push_back(FactOrCheck::getCheck(DTN, &U));
+      }
       continue;
     }
 
@@ -1260,14 +1267,12 @@ static void generateReproducer(CmpInst *Cond, Module *M,
   assert(!verifyFunction(*F, &dbgs()));
 }
 
-static std::optional<bool> checkCondition(CmpInst *Cmp, ConstraintInfo &Info,
+static std::optional<bool> checkCondition(CmpInst::Predicate Pred, Value *A,
+                                          Value *B, ConstraintInfo &Info,
                                           unsigned NumIn, unsigned NumOut,
                                           Instruction *ContextInst) {
-  LLVM_DEBUG(dbgs() << "Checking " << *Cmp << "\n");
-
-  CmpInst::Predicate Pred = Cmp->getPredicate();
-  Value *A = Cmp->getOperand(0);
-  Value *B = Cmp->getOperand(1);
+  LLVM_DEBUG(dbgs() << "Checking " << CmpInst::getPredicateName(Pred) << " "
+                    << *A << ", " << *B << "\n");
 
   auto R = Info.getConstraintForSolving(Pred, A, B);
   if (R.empty() || !R.isValid(Info)){
@@ -1293,9 +1298,10 @@ static std::optional<bool> checkCondition(CmpInst *Cmp, ConstraintInfo &Info,
 
     LLVM_DEBUG({
       if (*ImpliedCondition) {
-        dbgs() << "Condition " << *Cmp;
+        dbgs() << "Condition " << CmpInst::getPredicateName(Pred) << " " << *A
+               << ", " << *B;
       } else {
-        auto InversePred = Cmp->getInversePredicate();
+        auto InversePred = CmpInst::getInversePredicate(Pred);
         dbgs() << "Condition " << CmpInst::getPredicateName(InversePred) << " "
                << *A << ", " << *B;
       }
@@ -1339,11 +1345,53 @@ static bool checkAndReplaceCondition(
   };
 
   if (auto ImpliedCondition =
-          checkCondition(Cmp, Info, NumIn, NumOut, ContextInst))
+          checkCondition(Cmp->getPredicate(), Cmp->getOperand(0),
+                         Cmp->getOperand(1), Info, NumIn, NumOut, ContextInst))
     return ReplaceCmpWithConstant(Cmp, *ImpliedCondition);
   return false;
 }
 
+static bool checkAndReplaceMinMax(MinMaxIntrinsic *MinMax, ConstraintInfo &Info,
+                                  unsigned NumIn, unsigned NumOut,
+                                  Instruction *ContextInst,
+                                  Module *ReproducerModule,
+                                  ArrayRef<ReproducerEntry> ReproducerCondStack,
+                                  DominatorTree &DT,
+                                  SmallVectorImpl<Instruction *> &ToRemove) {
+  auto ReplaceMinMaxWithOperand = [&](MinMaxIntrinsic *MinMax, bool UseLHS) {
+    // TODO: generate reproducer for min/max.
+    MinMax->replaceUsesWithIf(MinMax->getOperand(UseLHS ? 0 : 1),
+                              [&DT, NumIn, NumOut, ContextInst](Use &U) {
+                                auto *UserI = getContextInstForUse(U);
+                                auto *DTN = DT.getNode(UserI->getParent());
+                                if (!DTN || DTN->getDFSNumIn() < NumIn ||
+                                    DTN->getDFSNumOut() > NumOut)
+                                  return false;
+                                if (UserI->getParent() ==
+                                        ContextInst->getParent() &&
+                                    UserI->comesBefore(ContextInst))
+                                  return false;
+
+                                return true;
+                              });
+    NumCondsRemoved++;
+    if (MinMax->use_empty())
+      ToRemove.push_back(MinMax);
+    return true;
+  };
+
+  if (auto ImpliedCondition = checkCondition(
+          MinMax->getPredicate(), MinMax->getOperand(0), MinMax->getOperand(1),
+          Info, NumIn, NumOut, ContextInst))
+    return ReplaceMinMaxWithOperand(MinMax, *ImpliedCondition);
+  if (auto ImpliedCondition = checkCondition(
+          ICmpInst::getNonStrictPredicate(MinMax->getPredicate()),
+          MinMax->getOperand(0), MinMax->getOperand(1), Info, NumIn, NumOut,
+          ContextInst))
+    return ReplaceMinMaxWithOperand(MinMax, *ImpliedCondition);
+  return false;
+}
+
 static void
 removeEntryFromStack(const StackEntry &E, ConstraintInfo &Info,
                      Module *ReproducerModule,
@@ -1380,9 +1428,10 @@ static bool checkAndSecondOpImpliedByFirst(
 
   bool Changed = false;
   // Check if the second condition can be simplified now.
-  if (auto ImpliedCondition =
-          checkCondition(cast<ICmpInst>(And->getOperand(1)), Info, CB.NumIn,
-                         CB.NumOut, CB.getContextInst())) {
+  ICmpInst *Cmp = cast<ICmpInst>(And->getOperand(1));
+  if (auto ImpliedCondition = checkCondition(
+          Cmp->getPredicate(), Cmp->getOperand(0), Cmp->getOperand(1), Info,
+          CB.NumIn, CB.NumOut, CB.getContextInst())) {
     And->setOperand(1, ConstantInt::getBool(And->getType(), *ImpliedCondition));
     Changed = true;
   }
@@ -1611,6 +1660,10 @@ static bool eliminateConstraints(Function &F, DominatorTree &DT, LoopInfo &LI,
                                              ReproducerCondStack, DFSInStack);
         }
         Changed |= Simplified;
+      } else if (auto *MinMax = dyn_cast<MinMaxIntrinsic>(Inst)) {
+        Changed |= checkAndReplaceMinMax(
+            MinMax, Info, CB.NumIn, CB.NumOut, CB.getContextInst(),
+            ReproducerModule.get(), ReproducerCondStack, S.DT, ToRemove);
       }
       continue;
     }
diff --git a/llvm/test/Transforms/ConstraintElimination/debug.ll b/llvm/test/Transforms/ConstraintElimination/debug.ll
index f3f0f5056135c2..a6ab609752a831 100644
--- a/llvm/test/Transforms/ConstraintElimination/debug.ll
+++ b/llvm/test/Transforms/ConstraintElimination/debug.ll
@@ -11,8 +11,8 @@ define i1 @test_and_ule(i4 %x, i4 %y, i4 %z) {
 ; CHECK-NEXT: Adding 'ule %y, %z'
 ; CHECK-NEXT:  constraint: %y + -1 * %z <= 0
 
-; CHECK: Checking   %t.1 = icmp ule i4 %x, %z
-; CHECK: Condition   %t.1 = icmp ule i4 %x, %z implied by dominating constraints
+; CHECK: Checking ule i4 %x, i4 %z
+; CHECK: Condition ule i4 %x, i4 %z implied by dominating constraints
 
 ; CHECK: Removing %y + -1 * %z <= 0
 ; CHECK: Removing %x + -1 * %y <= 0
@@ -41,7 +41,7 @@ define i1 @test_and_ugt(i4 %x, i4 %y, i4 %z) {
 ; CHECK-NEXT: Adding 'ugt %y, %z'
 ; CHECK-NEXT:  constraint: -1 * %y + %z <= -1
 
-; CHECK: Checking   %f.1 = icmp ule i4 %x, %z
+; CHECK: Checking ule i4 %x, i4 %z
 ; CHECK: Condition ugt i4 %x, i4 %z implied by dominating constraints
 
 ; CHECK: Removing -1 * %y + %z <= -1
diff --git a/llvm/test/Transforms/ConstraintElimination/minmax.ll b/llvm/test/Transforms/ConstraintElimination/minmax.ll
index a31cf6845ad67d..44751549acfad4 100644
--- a/llvm/test/Transforms/ConstraintElimination/minmax.ll
+++ b/llvm/test/Transforms/ConstraintElimination/minmax.ll
@@ -341,6 +341,147 @@ end:
   ret i32 0
 }
 
+; Test from PR75155
+define i32 @simplify_smax_val(i32 %a, i32 %b) {
+; CHECK-LABEL: define i32 @simplify_smax_val
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK:       then:
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[A]], 1
+; CHECK-NEXT:    ret i32 [[B]]
+; CHECK:       else:
+; CHECK-NEXT:    ret i32 -1
+;
+start:
+  %cmp = icmp slt i32 %a, %b
+  br i1 %cmp, label %then, label %else
+then:
+  %add = add nsw i32 %a, 1
+  %max = call i32 @llvm.smax.i32(i32 %b, i32 %add)
+  ret i32 %max
+else:
+  ret i32 -1
+}
+
+define i32 @simplify_umax_val(i32 %a, i32 %b) {
+; CHECK-LABEL: define i32 @simplify_umax_val
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK:       then:
+; CHECK-NEXT:    [[ADD:%.*]] = add nuw i32 [[A]], 1
+; CHECK-NEXT:    ret i32 [[B]]
+; CHECK:       else:
+; CHECK-NEXT:    ret i32 -1
+;
+start:
+  %cmp = icmp ult i32 %a, %b
+  br i1 %cmp, label %then, label %else
+then:
+  %add = add nuw i32 %a, 1
+  %max = call i32 @llvm.umax.i32(i32 %b, i32 %add)
+  ret i32 %max
+else:
+  ret i32 -1
+}
+
+define i32 @simplify_smin_val(i32 %a, i32 %b) {
+; CHECK-LABEL: define i32 @simplify_smin_val
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK:       then:
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[A]], 1
+; CHECK-NEXT:    ret i32 [[ADD]]
+; CHECK:       else:
+; CHECK-NEXT:    ret i32 -1
+;
+start:
+  %cmp = icmp slt i32 %a, %b
+  br i1 %cmp, label %then, label %else
+then:
+  %add = add nsw i32 %a, 1
+  %max = call i32 @llvm.smin.i32(i32 %b, i32 %add)
+  ret i32 %max
+else:
+  ret i32 -1
+}
+
+define i32 @simplify_umin_val(i32 %a, i32 %b) {
+; CHECK-LABEL: define i32 @simplify_umin_val
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK:       then:
+; CHECK-NEXT:    [[ADD:%.*]] = add nuw i32 [[A]], 1
+; CHECK-NEXT:    ret i32 [[ADD]]
+; CHECK:       else:
+; CHECK-NEXT:    ret i32 -1
+;
+start:
+  %cmp = icmp ult i32 %a, %b
+  br i1 %cmp, label %then, label %else
+then:
+  %add = add nuw i32 %a, 1
+  %max = call i32 @llvm.umin.i32(i32 %b, i32 %add)
+  ret i32 %max
+else:
+  ret i32 -1
+}
+
+define i32 @simplify_smax_val_fail1(i32 %a, i32 %b) {
+; CHECK-LABEL: define i32 @simplify_smax_val_fail1
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK:       then:
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[A]], 2
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.smax.i32(i32 [[B]], i32 [[ADD]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+; CHECK:       else:
+; CHECK-NEXT:    ret i32 -1
+;
+start:
+  %cmp = icmp slt i32 %a, %b
+  br i1 %cmp, label %then, label %else
+then:
+  %add = add nsw i32 %a, 2
+  %max = call i32 @llvm.smax.i32(i32 %b, i32 %add)
+  ret i32 %max
+else:
+  ret i32 -1
+}
+
+define i32 @simplify_smax_val_fail2(i32 %a, i32 %b) {
+; CHECK-LABEL: define i32 @simplify_smax_val_fail2
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) {
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK:       then:
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[A]], 1
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.smax.i32(i32 [[B]], i32 [[ADD]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+; CHECK:       else:
+; CHECK-NEXT:    ret i32 -1
+;
+start:
+  %cmp = icmp ult i32 %a, %b
+  br i1 %cmp, label %then, label %else
+then:
+  %add = add nsw i32 %a, 1
+  %max = call i32 @llvm.smax.i32(i32 %b, i32 %add)
+  ret i32 %max
+else:
+  ret i32 -1
+}
+
 declare i32 @llvm.smin.i32(i32, i32)
 declare i32 @llvm.smax.i32(i32, i32)
 declare i32 @llvm.umin.i32(i32, i32)
diff --git a/llvm/test/Transforms/ConstraintElimination/reproducer-remarks-debug.ll b/llvm/test/Transforms/ConstraintElimination/reproducer-remarks-debug.ll
index b8343aed8b4af7..b4b2e2ee7077ee 100644
--- a/llvm/test/Transforms/ConstraintElimination/reproducer-remarks-debug.ll
+++ b/llvm/test/Transforms/ConstraintElimination/reproducer-remarks-debug.ll
@@ -4,7 +4,7 @@
 
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
-; CHECK:      Condition   %c.2 = icmp eq ptr %a, null implied by dominating constraints
+; CHECK:      Condition eq ptr %a, ptr null implied by dominating constraints
 ; CHECK-NEXT: %a <= 0
 ; CHECK-NEXT: Creating reproducer for   %c.2 = icmp eq ptr %a, null
 ; CHECK-NEXT:   found external input ptr %a

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

It would be good to split this up into 3 parts at least

  1. precommit tests
  2. refactor checkCondition interface
  3. non-strict predicate support
    potentially 4) optimize MinMaxIntrinsics at uses

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp Outdated Show resolved Hide resolved
llvm/test/Transforms/ConstraintElimination/debug.ll Outdated Show resolved Hide resolved
llvm/lib/Transforms/Scalar/ConstraintElimination.cpp Outdated Show resolved Hide resolved
dtcxzyw added a commit that referenced this pull request Dec 13, 2023
This patch refactors `checkCondition` to handle min/max intrinsic calls
in #75306.
@fhahn
Copy link
Contributor

fhahn commented Dec 13, 2023

Thanks for the updates. It's a bit hard to see what's going on now as there are multiple commits in this PR now.

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Dec 14, 2023

Thanks for the updates. It's a bit hard to see what's going on now as there are multiple commits in this PR now.

I will combine the last two commits into one commit.

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Dec 23, 2023
@dtcxzyw dtcxzyw requested a review from fhahn December 26, 2023 22:09
@fhahn
Copy link
Contributor

fhahn commented Jan 4, 2024

Looks like this needs a rebase now

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jan 8, 2024

Looks like this needs a rebase now

Done.

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update!

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@dtcxzyw dtcxzyw merged commit bc9c2be into llvm:main Feb 4, 2024
4 checks passed
@dtcxzyw dtcxzyw deleted the constraintelim-minmax branch February 4, 2024 13:07
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
This patch replaces min/max intrinsic with one of its operands if
possible.
Alive2: https://alive2.llvm.org/ce/z/LoHfYf
Fixes llvm#75155.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing optimization: fold max(b, a + 1) to b when a < b
4 participants