Skip to content

Recommit "[ConstraintElim] Simplify cmp after uadd.sat/usub.sat (#135603)" #136467

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

Conversation

el-ev
Copy link
Member

@el-ev el-ev commented Apr 20, 2025

No description provided.

Copy link
Member Author

el-ev commented Apr 20, 2025

@el-ev el-ev marked this pull request as ready for review April 20, 2025 02:34
@el-ev el-ev changed the title "Recommit "[ConstraintElim] Simplify cmp after uadd.sat/usub.sat (#135603)" Recommit "[ConstraintElim] Simplify cmp after uadd.sat/usub.sat (#135603)" Apr 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 20, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Iris (el-ev)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+16)
  • (modified) llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll (+2-5)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 8684420b73671..9fd6b067debdf 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1139,6 +1139,8 @@ void State::addInfoFor(BasicBlock &BB) {
       // when simplifying uses of the min/max intrinsics.
       [[fallthrough]];
     case Intrinsic::abs:
+    case Intrinsic::uadd_sat:
+    case Intrinsic::usub_sat:
       if (!isGuaranteedNotToBePoison(&I))
         break;
       WorkList.push_back(FactOrCheck::getInstFact(DT.getNode(&BB), &I));
@@ -1896,6 +1898,20 @@ static bool eliminateConstraints(Function &F, DominatorTree &DT, LoopInfo &LI,
         AddFact(Pred, MinMax, MinMax->getRHS());
         continue;
       }
+      if (auto *USatI = dyn_cast<SaturatingInst>(CB.Inst)) {
+        switch (USatI->getIntrinsicID()) {
+        default:
+          llvm_unreachable("Unexpected intrinsic.");
+        case Intrinsic::uadd_sat:
+          AddFact(ICmpInst::ICMP_UGE, USatI, USatI->getLHS());
+          AddFact(ICmpInst::ICMP_UGE, USatI, USatI->getRHS());
+          break;
+        case Intrinsic::usub_sat:
+          AddFact(ICmpInst::ICMP_ULE, USatI, USatI->getLHS());
+          break;
+        }
+        continue;
+      }
     }
 
     Value *A = nullptr, *B = nullptr;
diff --git a/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll b/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll
index 42bbe67024e58..c19a0548d78b2 100644
--- a/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll
+++ b/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll
@@ -8,9 +8,7 @@ define i1 @uadd_sat_uge(i64 noundef %a, i64 noundef %b) {
 ; CHECK-LABEL: define i1 @uadd_sat_uge(
 ; CHECK-SAME: i64 noundef [[A:%.*]], i64 noundef [[B:%.*]]) {
 ; CHECK-NEXT:    [[ADD_SAT:%.*]] = call i64 @llvm.uadd.sat.i64(i64 [[A]], i64 [[B]])
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp uge i64 [[ADD_SAT]], [[A]]
-; CHECK-NEXT:    [[CMP2:%.*]] = icmp uge i64 [[ADD_SAT]], [[B]]
-; CHECK-NEXT:    [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    [[CMP:%.*]] = and i1 true, true
 ; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %add.sat = call i64 @llvm.uadd.sat.i64(i64 %a, i64 %b)
@@ -24,8 +22,7 @@ define i1 @usub_sat_ule_lhs(i64 noundef %a, i64 noundef %b) {
 ; CHECK-LABEL: define i1 @usub_sat_ule_lhs(
 ; CHECK-SAME: i64 noundef [[A:%.*]], i64 noundef [[B:%.*]]) {
 ; CHECK-NEXT:    [[SUB_SAT:%.*]] = call i64 @llvm.usub.sat.i64(i64 [[A]], i64 [[B]])
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ule i64 [[SUB_SAT]], [[A]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 true
 ;
   %sub.sat = call i64 @llvm.usub.sat.i64(i64 %a, i64 %b)
   %cmp = icmp ule i64 %sub.sat, %a

el-ev added 4 commits April 30, 2025 14:37
…re_adding_intrinsic_facts' into users/el-ev/04-20-_recommit_constraintelim_simplify_cmp_after_uadd.sat_usub.sat_135603_
…re_adding_intrinsic_facts' into users/el-ev/04-20-_recommit_constraintelim_simplify_cmp_after_uadd.sat_usub.sat_135603_
Copy link
Member Author

el-ev commented Apr 30, 2025

Merge activity

  • Apr 30, 9:40 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 30, 9:42 AM EDT: Graphite couldn't merge this pull request because a downstack PR [ConstraintElim] Fix poison check before adding intrinsic facts #136291 failed to merge.
  • Apr 30, 9:44 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 30, 9:47 AM EDT: Graphite couldn't merge this PR because it had conflicts with the trunk branch.
  • Apr 30, 9:47 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 30, 9:49 AM EDT: @el-ev merged this pull request with Graphite.

Base automatically changed from users/el-ev/04-18-_constraintelim_fix_poison_check_before_adding_intrinsic_facts to main April 30, 2025 13:46
@el-ev el-ev merged commit b54b3e8 into main Apr 30, 2025
6 of 9 checks passed
@el-ev el-ev deleted the users/el-ev/04-20-_recommit_constraintelim_simplify_cmp_after_uadd.sat_usub.sat_135603_ branch April 30, 2025 13:49
@el-ev el-ev restored the users/el-ev/04-20-_recommit_constraintelim_simplify_cmp_after_uadd.sat_usub.sat_135603_ branch April 30, 2025 13:49
@el-ev el-ev deleted the users/el-ev/04-20-_recommit_constraintelim_simplify_cmp_after_uadd.sat_usub.sat_135603_ branch April 30, 2025 13:50
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
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.

3 participants