Skip to content

Commit

Permalink
[DivRemPairs] Do not freeze poisons that can't be undef (#92627)
Browse files Browse the repository at this point in the history
Per comments in DivRemPairs, the rewrite from
```llvm
    %div = div %X, %Y
    %rem = rem %X, %Y
```
to
```llvm
    %div = div %X, %Y
    %.mul = mul %div, %Y
    %rem = sub %X, %mul
```
is unsound when %X or %Y are undef.

However, it is known to be sound if %X or %Y are poison but can't be
undef, since both the pre- and post-rewrite %rem are `poison`.

Additionally, proofs: https://alive2.llvm.org/ce/z/xtNQ8j

A comment in the pass listed a TODO for changing a usage of
isGuaranteedNotToBeUndefOrPoison() in the pass to something that only
detects undef. Such a function has been implemented since the time that
TODO was written, but has not been used.

Therefore, this commit updates DivRemPairs to use
isGuaranteedNotToBeUndef() instead.
  • Loading branch information
krzysz00 authored May 20, 2024
1 parent 02f1a99 commit 8b22bb8
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 30 deletions.
5 changes: 2 additions & 3 deletions llvm/lib/Transforms/Scalar/DivRemPairs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -381,16 +381,15 @@ static bool optimizeDivRem(Function &F, const TargetTransformInfo &TTI,
// %mul = mul %div, 1 // %mul = undef
// %rem = sub %x, %mul // %rem = undef - undef = undef
// If X is not frozen, %rem becomes undef after transformation.
// TODO: We need a undef-specific checking function in ValueTracking
if (!isGuaranteedNotToBeUndefOrPoison(X, nullptr, DivInst, &DT)) {
if (!isGuaranteedNotToBeUndef(X, nullptr, DivInst, &DT)) {
auto *FrX =
new FreezeInst(X, X->getName() + ".frozen", DivInst->getIterator());
DivInst->setOperand(0, FrX);
Sub->setOperand(0, FrX);
}
// Same for Y. If X = 1 and Y = (undef | 1), %rem in src is either 1 or 0,
// but %rem in tgt can be one of many integer values.
if (!isGuaranteedNotToBeUndefOrPoison(Y, nullptr, DivInst, &DT)) {
if (!isGuaranteedNotToBeUndef(Y, nullptr, DivInst, &DT)) {
auto *FrY =
new FreezeInst(Y, Y->getName() + ".frozen", DivInst->getIterator());
DivInst->setOperand(1, FrY);
Expand Down
42 changes: 15 additions & 27 deletions llvm/test/Transforms/DivRemPairs/AMDGPU/div-rem-pairs.ll
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,14 @@ define i32 @no_freezes(ptr %p, i32 noundef %x, i32 noundef %y) {
ret i32 %rem
}

; FIXME: There should be no need to `freeze` x2 and y2 since they have defined
; but potentially poison values.
define i32 @poison_does_not_freeze(ptr %p, i32 noundef %x, i32 noundef %y) {
; CHECK-LABEL: define i32 @poison_does_not_freeze(
; CHECK-SAME: ptr [[P:%.*]], i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) {
; CHECK-NEXT: [[X2:%.*]] = shl nuw nsw i32 [[X]], 5
; CHECK-NEXT: [[Y2:%.*]] = add nuw nsw i32 [[Y]], 1
; CHECK-NEXT: [[X2_FROZEN:%.*]] = freeze i32 [[X2]]
; CHECK-NEXT: [[Y2_FROZEN:%.*]] = freeze i32 [[Y2]]
; CHECK-NEXT: [[DIV:%.*]] = udiv i32 [[X2_FROZEN]], [[Y2_FROZEN]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2_FROZEN]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X2_FROZEN]], [[TMP1]]
; CHECK-NEXT: [[DIV:%.*]] = udiv i32 [[X2]], [[Y2]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X2]], [[TMP1]]
; CHECK-NEXT: store i32 [[DIV]], ptr [[P]], align 4
; CHECK-NEXT: ret i32 [[REM_DECOMPOSED]]
;
Expand All @@ -61,11 +57,9 @@ define i32 @poison_does_not_freeze_signed(ptr %p, i32 noundef %x, i32 noundef %y
; CHECK-SAME: ptr [[P:%.*]], i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) {
; CHECK-NEXT: [[X2:%.*]] = shl nuw nsw i32 [[X]], 5
; CHECK-NEXT: [[Y2:%.*]] = add nuw nsw i32 [[Y]], 1
; CHECK-NEXT: [[X2_FROZEN:%.*]] = freeze i32 [[X2]]
; CHECK-NEXT: [[Y2_FROZEN:%.*]] = freeze i32 [[Y2]]
; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[X2_FROZEN]], [[Y2_FROZEN]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2_FROZEN]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X2_FROZEN]], [[TMP1]]
; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[X2]], [[Y2]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X2]], [[TMP1]]
; CHECK-NEXT: store i32 [[DIV]], ptr [[P]], align 4
; CHECK-NEXT: ret i32 [[REM_DECOMPOSED]]
;
Expand All @@ -82,11 +76,9 @@ define <4 x i8> @poison_does_not_freeze_vector(ptr %p, <4 x i8> noundef %x, <4 x
; CHECK-SAME: ptr [[P:%.*]], <4 x i8> noundef [[X:%.*]], <4 x i8> noundef [[Y:%.*]]) {
; CHECK-NEXT: [[X2:%.*]] = shl nuw nsw <4 x i8> [[X]], <i8 5, i8 5, i8 5, i8 5>
; CHECK-NEXT: [[Y2:%.*]] = add nuw nsw <4 x i8> [[Y]], <i8 1, i8 1, i8 1, i8 1>
; CHECK-NEXT: [[X2_FROZEN:%.*]] = freeze <4 x i8> [[X2]]
; CHECK-NEXT: [[Y2_FROZEN:%.*]] = freeze <4 x i8> [[Y2]]
; CHECK-NEXT: [[DIV:%.*]] = udiv <4 x i8> [[X2_FROZEN]], [[Y2_FROZEN]]
; CHECK-NEXT: [[TMP1:%.*]] = mul <4 x i8> [[DIV]], [[Y2_FROZEN]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub <4 x i8> [[X2_FROZEN]], [[TMP1]]
; CHECK-NEXT: [[DIV:%.*]] = udiv <4 x i8> [[X2]], [[Y2]]
; CHECK-NEXT: [[TMP1:%.*]] = mul <4 x i8> [[DIV]], [[Y2]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub <4 x i8> [[X2]], [[TMP1]]
; CHECK-NEXT: store <4 x i8> [[DIV]], ptr [[P]], align 4
; CHECK-NEXT: ret <4 x i8> [[REM_DECOMPOSED]]
;
Expand All @@ -103,11 +95,9 @@ define i32 @explicit_poison_does_not_freeze(ptr %p, i32 noundef %y) {
; CHECK-SAME: ptr [[P:%.*]], i32 noundef [[Y:%.*]]) {
; CHECK-NEXT: [[X:%.*]] = add i32 poison, 1
; CHECK-NEXT: [[Y2:%.*]] = add nuw nsw i32 [[Y]], 1
; CHECK-NEXT: [[X_FROZEN:%.*]] = freeze i32 [[X]]
; CHECK-NEXT: [[Y2_FROZEN:%.*]] = freeze i32 [[Y2]]
; CHECK-NEXT: [[DIV:%.*]] = udiv i32 [[X_FROZEN]], [[Y2_FROZEN]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2_FROZEN]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X_FROZEN]], [[TMP1]]
; CHECK-NEXT: [[DIV:%.*]] = udiv i32 [[X]], [[Y2]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X]], [[TMP1]]
; CHECK-NEXT: store i32 [[DIV]], ptr [[P]], align 4
; CHECK-NEXT: ret i32 [[REM_DECOMPOSED]]
;
Expand All @@ -124,11 +114,9 @@ define i32 @explicit_poison_does_not_freeze_signed(ptr %p, i32 noundef %y) {
; CHECK-SAME: ptr [[P:%.*]], i32 noundef [[Y:%.*]]) {
; CHECK-NEXT: [[X:%.*]] = add i32 poison, 1
; CHECK-NEXT: [[Y2:%.*]] = add nuw nsw i32 [[Y]], 1
; CHECK-NEXT: [[X_FROZEN:%.*]] = freeze i32 [[X]]
; CHECK-NEXT: [[Y2_FROZEN:%.*]] = freeze i32 [[Y2]]
; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[X_FROZEN]], [[Y2_FROZEN]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2_FROZEN]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X_FROZEN]], [[TMP1]]
; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[X]], [[Y2]]
; CHECK-NEXT: [[TMP1:%.*]] = mul i32 [[DIV]], [[Y2]]
; CHECK-NEXT: [[REM_DECOMPOSED:%.*]] = sub i32 [[X]], [[TMP1]]
; CHECK-NEXT: store i32 [[DIV]], ptr [[P]], align 4
; CHECK-NEXT: ret i32 [[REM_DECOMPOSED]]
;
Expand Down

0 comments on commit 8b22bb8

Please sign in to comment.