Skip to content

Commit

Permalink
[ConstraintElim] Make sure min/max intrinsic results are not poison.
Browse files Browse the repository at this point in the history
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 #78621.

(cherry picked from commit 3d91d96)
  • Loading branch information
fhahn authored and tstellar committed Feb 7, 2024
1 parent 4251261 commit 56c50a4
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 4 deletions.
7 changes: 6 additions & 1 deletion llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion llvm/test/Transforms/ConstraintElimination/minmax.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 56c50a4

Please sign in to comment.