[ConstraintElim] Fix poison check before adding intrinsic facts #136291

el-ev · 2025-04-18T10:56:56Z

No description provided.

el-ev · 2025-04-18T10:57:13Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-04-18T10:57:47Z

@llvm/pr-subscribers-llvm-transforms

Author: Iris (el-ev)

Changes

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

3 Files Affected:

(modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+2-2)
(modified) llvm/test/Transforms/ConstraintElimination/abs.ll (+16-8)
(modified) llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll (+5-2)

diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 31e057baee2bf..75e5ccd1a72e7 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1137,12 +1137,12 @@ void State::addInfoFor(BasicBlock &BB) {
           FactOrCheck::getCheck(DT.getNode(&BB), cast<CallInst>(&I)));
       // TODO: Check if it is possible to instead only added the min/max facts
       // when simplifying uses of the min/max intrinsics.
-      if (!isGuaranteedNotToBePoison(&I))
-        break;
       [[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));
       break;
     }
diff --git a/llvm/test/Transforms/ConstraintElimination/abs.ll b/llvm/test/Transforms/ConstraintElimination/abs.ll
index 9fc68b0e72663..a0c22d9f03c28 100644
--- a/llvm/test/Transforms/ConstraintElimination/abs.ll
+++ b/llvm/test/Transforms/ConstraintElimination/abs.ll
@@ -5,7 +5,8 @@ define i1 @abs_int_min_is_not_poison(i32 %arg) {
 ; CHECK-LABEL: define i1 @abs_int_min_is_not_poison(
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 false)
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sge i32 [[ABS]], [[ARG]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 false)
   %cmp = icmp sge i32 %abs, %arg
@@ -16,7 +17,8 @@ define i1 @abs_int_min_is_poison(i32 %arg) {
 ; CHECK-LABEL: define i1 @abs_int_min_is_poison(
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 true)
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sge i32 [[ABS]], [[ARG]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 true)
   %cmp = icmp sge i32 %abs, %arg
@@ -28,7 +30,8 @@ define i1 @abs_plus_one(i32 %arg) {
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 true)
 ; CHECK-NEXT:    [[ABS_PLUS_ONE:%.*]] = add nsw i32 [[ABS]], 1
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sge i32 [[ABS_PLUS_ONE]], [[ARG]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 true)
   %abs_plus_one = add nsw i32 %abs, 1
@@ -41,7 +44,8 @@ define i1 @arg_minus_one_strict_less(i32 %arg) {
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 true)
 ; CHECK-NEXT:    [[ARG_MINUS_ONE:%.*]] = add nsw i32 [[ARG]], -1
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[ARG_MINUS_ONE]], [[ABS]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 true)
   %arg_minus_one = add nsw i32 %arg, -1
@@ -54,7 +58,8 @@ define i1 @arg_minus_one_strict_greater(i32 %arg) {
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 true)
 ; CHECK-NEXT:    [[ARG_MINUS_ONE:%.*]] = add nsw i32 [[ARG]], -1
-; CHECK-NEXT:    ret i1 false
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[ARG_MINUS_ONE]], [[ABS]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 true)
   %arg_minus_one = add nsw i32 %arg, -1
@@ -69,7 +74,8 @@ define i1 @abs_plus_one_unsigned_greater_or_equal_nonnegative_arg(i32 %arg) {
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[CMP_ARG_NONNEGATIVE]])
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 true)
 ; CHECK-NEXT:    [[ABS_PLUS_ONE:%.*]] = add nuw i32 [[ABS]], 1
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp uge i32 [[ABS_PLUS_ONE]], [[ARG]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %cmp_arg_nonnegative = icmp sge i32 %arg, 0
   call void @llvm.assume(i1 %cmp_arg_nonnegative)
@@ -142,7 +148,8 @@ define i1 @abs_is_nonnegative_int_min_is_poison(i32 %arg) {
 ; CHECK-LABEL: define i1 @abs_is_nonnegative_int_min_is_poison(
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 [[ARG]], i1 true)
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sge i32 [[ABS]], 0
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 true)
   %cmp = icmp sge i32 %abs, 0
@@ -152,7 +159,8 @@ define i1 @abs_is_nonnegative_int_min_is_poison(i32 %arg) {
 define i1 @abs_is_nonnegative_constant_arg() {
 ; CHECK-LABEL: define i1 @abs_is_nonnegative_constant_arg() {
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call i32 @llvm.abs.i32(i32 -3, i1 true)
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sge i32 [[ABS]], 0
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %abs = tail call i32 @llvm.abs.i32(i32 -3, i1 true)
   %cmp = icmp sge i32 %abs, 0
diff --git a/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll b/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll
index b0db89dcfdab8..6feb87c49f406 100644
--- a/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll
+++ b/llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll
@@ -8,7 +8,9 @@ define i1 @uadd_sat_uge(i64 %a, i64 %b) {
 ; CHECK-LABEL: define i1 @uadd_sat_uge(
 ; CHECK-SAME: i64 [[A:%.*]], i64 [[B:%.*]]) {
 ; CHECK-NEXT:    [[ADD_SAT:%.*]] = call i64 @llvm.uadd.sat.i64(i64 [[A]], i64 [[B]])
-; CHECK-NEXT:    [[CMP:%.*]] = and i1 true, true
+; 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:    ret i1 [[CMP]]
 ;
   %add.sat = call i64 @llvm.uadd.sat.i64(i64 %a, i64 %b)
@@ -22,7 +24,8 @@ define i1 @usub_sat_ule_lhs(i64 %a, i64 %b) {
 ; CHECK-LABEL: define i1 @usub_sat_ule_lhs(
 ; CHECK-SAME: i64 [[A:%.*]], i64 [[B:%.*]]) {
 ; CHECK-NEXT:    [[SUB_SAT:%.*]] = call i64 @llvm.usub.sat.i64(i64 [[A]], i64 [[B]])
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ule i64 [[SUB_SAT]], [[A]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %sub.sat = call i64 @llvm.usub.sat.i64(i64 %a, i64 %b)
   %cmp = icmp ule i64 %sub.sat, %a

nikic

We should add noundef rather than updating the tests.

llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll

el-ev · 2025-04-25T03:21:05Z

ping

…_check_before_adding_intrinsic_facts

nikic

I was hoping that we would have a proper solution (#80121, #136558) to this soon.

Extending this check to abs is problematic due to the interaction with the int_min_is_poison flag.

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp

el-ev · 2025-04-30T09:29:01Z

Extending this check to abs is problematic due to the interaction with the int_min_is_poison flag.

Then there's no functional change in this patch.

github-actions · 2025-04-30T09:31:12Z

✅ With the latest revision this PR passed the C/C++ code formatter.

nikic · 2025-04-30T13:08:00Z

Extending this check to abs is problematic due to the interaction with the int_min_is_poison flag.

Then there's no functional change in this patch.

Right. I think it's okay to add the poison check workaround for usub_sat to unblock the patch stack, but I don't think we should touch abs until we can handle this properly.

el-ev · 2025-04-30T13:40:14Z

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 PR because it had conflicts with the trunk branch.
Apr 30, 9:44 AM EDT: A user started a stack merge that includes this pull request via Graphite.
Apr 30, 9:46 AM EDT: @el-ev merged this pull request with Graphite.

…_check_before_adding_intrinsic_facts

…#136291)

el-ev marked this pull request as ready for review April 18, 2025 10:57

llvmbot added the llvm:transforms label Apr 18, 2025

el-ev requested a review from nikic April 18, 2025 10:59

nikic reviewed Apr 19, 2025

View reviewed changes

llvm/test/Transforms/ConstraintElimination/uadd-usub-sat.ll Outdated Show resolved Hide resolved

el-ev force-pushed the users/el-ev/04-18-_constraintelim_fix_poison_check_before_adding_intrinsic_facts branch from dd33e67 to cdb1065 Compare April 20, 2025 02:27

[ConstraintElim] Fix poison check before adding intrinsic facts

f4bf37b

el-ev force-pushed the users/el-ev/04-18-_constraintelim_fix_poison_check_before_adding_intrinsic_facts branch from cdb1065 to f4bf37b Compare April 20, 2025 02:30

This was referenced Apr 20, 2025

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

Merged

[ConstraintElim] Add noundef to several testcases (NFC) #135799

Closed

el-ev requested a review from nikic April 22, 2025 01:19

Merge branch 'main' into users/el-ev/04-18-_constraintelim_fix_poison…

3e63609

…_check_before_adding_intrinsic_facts

dtcxzyw approved these changes Apr 30, 2025

View reviewed changes

nikic reviewed Apr 30, 2025

View reviewed changes

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp Outdated Show resolved Hide resolved

update

4a30bfe

el-ev force-pushed the users/el-ev/04-18-_constraintelim_fix_poison_check_before_adding_intrinsic_facts branch from dbf9e29 to 4a30bfe Compare April 30, 2025 09:33

Merge branch 'main' into users/el-ev/04-18-_constraintelim_fix_poison…

20b12a7

…_check_before_adding_intrinsic_facts

el-ev merged commit 349dc34 into main Apr 30, 2025
6 of 9 checks passed

el-ev deleted the users/el-ev/04-18-_constraintelim_fix_poison_check_before_adding_intrinsic_facts branch April 30, 2025 13:46

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025

[ConstraintElim] Fix poison check before adding intrinsic facts (llvm…

dcc1dcd

…#136291)

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025

[ConstraintElim] Fix poison check before adding intrinsic facts (llvm…

cc5e4e5

…#136291)

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025

[ConstraintElim] Fix poison check before adding intrinsic facts (llvm…

7698dc2

…#136291)

GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025

[ConstraintElim] Fix poison check before adding intrinsic facts (llvm…

639fd59

…#136291)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ConstraintElim] Fix poison check before adding intrinsic facts #136291

[ConstraintElim] Fix poison check before adding intrinsic facts #136291

el-ev commented Apr 18, 2025

el-ev commented Apr 18, 2025 •

edited

Loading

llvmbot commented Apr 18, 2025

nikic left a comment

el-ev commented Apr 25, 2025

nikic left a comment

el-ev commented Apr 30, 2025

github-actions bot commented Apr 30, 2025 •

edited

Loading

nikic commented Apr 30, 2025

el-ev commented Apr 30, 2025 •

edited

Loading

[ConstraintElim] Fix poison check before adding intrinsic facts #136291

[ConstraintElim] Fix poison check before adding intrinsic facts #136291

Conversation

el-ev commented Apr 18, 2025

el-ev commented Apr 18, 2025 • edited Loading

llvmbot commented Apr 18, 2025

nikic left a comment

Choose a reason for hiding this comment

el-ev commented Apr 25, 2025

nikic left a comment

Choose a reason for hiding this comment

el-ev commented Apr 30, 2025

github-actions bot commented Apr 30, 2025 • edited Loading

nikic commented Apr 30, 2025

el-ev commented Apr 30, 2025 • edited Loading

Merge activity

el-ev commented Apr 18, 2025 •

edited

Loading

github-actions bot commented Apr 30, 2025 •

edited

Loading

el-ev commented Apr 30, 2025 •

edited

Loading