[llvm] [DivRemPairs] Do not freeze poisons that can't be undef (PR #92627)
Krzysztof Drewniak via llvm-commits
llvm-commits at lists.llvm.org
Mon May 20 08:31:28 PDT 2024
https://github.com/krzysz00 updated https://github.com/llvm/llvm-project/pull/92627
>From 58c5147dd7f37dd30d0010208c5b549ef6776611 Mon Sep 17 00:00:00 2001
From: Krzysztof Drewniak <Krzysztof.Drewniak at amd.com>
Date: Sat, 18 May 2024 01:35:18 +0000
Subject: [PATCH] [DivRemPairs] Do not freeze poisons that can't be undef
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.
---
llvm/lib/Transforms/Scalar/DivRemPairs.cpp | 5 +--
.../DivRemPairs/AMDGPU/div-rem-pairs.ll | 42 +++++++------------
2 files changed, 17 insertions(+), 30 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
index 45f36a36b5dd0..f7ada9fb8eb8a 100644
--- a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
+++ b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
@@ -381,8 +381,7 @@ 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);
@@ -390,7 +389,7 @@ static bool optimizeDivRem(Function &F, const TargetTransformInfo &TTI,
}
// 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);
diff --git a/llvm/test/Transforms/DivRemPairs/AMDGPU/div-rem-pairs.ll b/llvm/test/Transforms/DivRemPairs/AMDGPU/div-rem-pairs.ll
index bd7a20a98539e..d01ded9ebbfda 100644
--- a/llvm/test/Transforms/DivRemPairs/AMDGPU/div-rem-pairs.ll
+++ b/llvm/test/Transforms/DivRemPairs/AMDGPU/div-rem-pairs.ll
@@ -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]]
;
@@ -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]]
;
@@ -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]]
;
@@ -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]]
;
@@ -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]]
;
More information about the llvm-commits
mailing list