[llvm-branch-commits] [llvm] 56c50a4 - [ConstraintElim] Make sure min/max intrinsic results are not poison.

[Tom Stellard via llvm-branch-commits]

Author: Florian Hahn
Date: 2024-02-06T16:05:16-08:00
New Revision: 56c50a4e0b50fb59ae1f1b322b31a5546522a928

URL: https://github.com/llvm/llvm-project/commit/56c50a4e0b50fb59ae1f1b322b31a5546522a928
DIFF: https://github.com/llvm/llvm-project/commit/56c50a4e0b50fb59ae1f1b322b31a5546522a928.diff

LOG: [ConstraintElim] Make sure min/max intrinsic results are not poison.

The result of umin may be poison and in that case the added constraints
are not be valid in contexts where poison doesn't cause UB. Only queue
facts for min/max intrinsics if the result is guaranteed to not be
poison.

This could be improved in the future, by only adding the fact when
solving conditions using the result value.

Fixes https://github.com/llvm/llvm-project/issues/78621.

(cherry picked from commit 3d91d9613e294b242d853039209b40a0cb7853f2)

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
    llvm/test/Transforms/ConstraintElimination/minmax.ll
    llvm/test/Transforms/ConstraintElimination/umin-result-may-be-poison.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 8f09569d0d9cc..7b672e89b67aa 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1061,11 +1061,16 @@ void State::addInfoFor(BasicBlock &BB) {
           FactOrCheck::getCheck(DT.getNode(&BB), cast<CallInst>(&I)));
       break;
     // Enqueue the intrinsics to add extra info.
-    case Intrinsic::abs:
     case Intrinsic::umin:
     case Intrinsic::umax:
     case Intrinsic::smin:
     case Intrinsic::smax:
+      // 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:
       WorkList.push_back(FactOrCheck::getInstFact(DT.getNode(&BB), &I));
       break;
     }

diff  --git a/llvm/test/Transforms/ConstraintElimination/minmax.ll b/llvm/test/Transforms/ConstraintElimination/minmax.ll
index a31cf6845ad67..82b932f14c4ff 100644
--- a/llvm/test/Transforms/ConstraintElimination/minmax.ll
+++ b/llvm/test/Transforms/ConstraintElimination/minmax.ll
@@ -306,7 +306,9 @@ define i1 @smin_branchless(i32 %x, i32 %y) {
 ; CHECK-SAME: (i32 [[X:%.*]], i32 [[Y:%.*]]) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.smin.i32(i32 [[X]], i32 [[Y]])
-; CHECK-NEXT:    [[RET:%.*]] = xor i1 true, false
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp sle i32 [[MIN]], [[X]]
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp sgt i32 [[MIN]], [[X]]
+; CHECK-NEXT:    [[RET:%.*]] = xor i1 [[CMP1]], [[CMP2]]
 ; CHECK-NEXT:    ret i1 [[RET]]
 ;
 entry:

diff  --git a/llvm/test/Transforms/ConstraintElimination/umin-result-may-be-poison.ll b/llvm/test/Transforms/ConstraintElimination/umin-result-may-be-poison.ll
index 35ac72e54d189..6d1d95ec4fdba 100644
--- a/llvm/test/Transforms/ConstraintElimination/umin-result-may-be-poison.ll
+++ b/llvm/test/Transforms/ConstraintElimination/umin-result-may-be-poison.ll
@@ -6,10 +6,11 @@
 define i1 @umin_not_used(i32 %arg) {
 ; CHECK-LABEL: define i1 @umin_not_used(
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
+; CHECK-NEXT:    [[ICMP:%.*]] = icmp slt i32 [[ARG]], 0
 ; CHECK-NEXT:    [[SHL:%.*]] = shl nuw nsw i32 [[ARG]], 3
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umin.i32(i32 [[SHL]], i32 80)
 ; CHECK-NEXT:    [[CMP2:%.*]] = shl nuw nsw i32 [[ARG]], 3
-; CHECK-NEXT:    ret i1 false
+; CHECK-NEXT:    ret i1 [[ICMP]]
 ;
   %icmp = icmp slt i32 %arg, 0
   %shl = shl nuw nsw i32 %arg, 3
@@ -38,12 +39,13 @@ define i1 @umin_poison_is_UB_via_call(i32 %arg) {
 define i1 @umin_poison_call_before_UB(i32 %arg) {
 ; CHECK-LABEL: define i1 @umin_poison_call_before_UB(
 ; CHECK-SAME: i32 [[ARG:%.*]]) {
+; CHECK-NEXT:    [[ICMP:%.*]] = icmp slt i32 [[ARG]], 0
 ; CHECK-NEXT:    [[SHL:%.*]] = shl nuw nsw i32 [[ARG]], 3
 ; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.umin.i32(i32 [[SHL]], i32 80)
 ; CHECK-NEXT:    call void @fn()
 ; CHECK-NEXT:    call void @noundef(i32 noundef [[MIN]])
 ; CHECK-NEXT:    [[CMP2:%.*]] = shl nuw nsw i32 [[ARG]], 3
-; CHECK-NEXT:    ret i1 false
+; CHECK-NEXT:    ret i1 [[ICMP]]
 ;
   %icmp = icmp slt i32 %arg, 0
   %shl = shl nuw nsw i32 %arg, 3