[llvm] f1c18ac - [NewGVN] do phi(undef, x) -> x only if x is not poison
Nuno Lopes via llvm-commits
llvm-commits at lists.llvm.org
Sat Jan 29 13:44:21 PST 2022
Author: Nuno Lopes
Date: 2022-01-29T21:43:57Z
New Revision: f1c18acb07aa40f42b87b70462a6d1ab77a4825c
URL: https://github.com/llvm/llvm-project/commit/f1c18acb07aa40f42b87b70462a6d1ab77a4825c
DIFF: https://github.com/llvm/llvm-project/commit/f1c18acb07aa40f42b87b70462a6d1ab77a4825c.diff
LOG: [NewGVN] do phi(undef, x) -> x only if x is not poison
phi([undef, A], [x, B]) -> x is only correct x is guaranteed to be
a non-poison value.
Otherwise we would be changing an undef to poison in the branch A.
Differential Revision: https://reviews.llvm.org/D117907
Added:
Modified:
llvm/lib/Transforms/Scalar/NewGVN.cpp
llvm/test/Transforms/NewGVN/completeness.ll
llvm/test/Transforms/NewGVN/phi-edge-handling.ll
llvm/test/Transforms/NewGVN/pr31682.ll
llvm/test/Transforms/NewGVN/pr32403.ll
llvm/test/Transforms/NewGVN/pr32838.ll
llvm/test/Transforms/NewGVN/pr34135.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 2476e6c408b1d..f35c9212a6f9e 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -77,6 +77,7 @@
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/MemorySSA.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/Argument.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/Constant.h"
@@ -1736,18 +1737,18 @@ NewGVN::performSymbolicPHIEvaluation(ArrayRef<ValPair> PHIOps,
if (Filtered.empty()) {
// If it has undef or poison at this point, it means there are no-non-undef
// arguments, and thus, the value of the phi node must be undef.
- if (HasPoison && !HasUndef) {
- LLVM_DEBUG(
- dbgs() << "PHI Node " << *I
- << " has no non-poison arguments, valuing it as poison\n");
- return createConstantExpression(PoisonValue::get(I->getType()));
- }
if (HasUndef) {
LLVM_DEBUG(
dbgs() << "PHI Node " << *I
<< " has no non-undef arguments, valuing it as undef\n");
return createConstantExpression(UndefValue::get(I->getType()));
}
+ if (HasPoison) {
+ LLVM_DEBUG(
+ dbgs() << "PHI Node " << *I
+ << " has no non-poison arguments, valuing it as poison\n");
+ return createConstantExpression(PoisonValue::get(I->getType()));
+ }
LLVM_DEBUG(dbgs() << "No arguments of PHI node " << *I << " are live\n");
deleteExpression(E);
@@ -1757,6 +1758,11 @@ NewGVN::performSymbolicPHIEvaluation(ArrayRef<ValPair> PHIOps,
++Filtered.begin();
// Can't use std::equal here, sadly, because filter.begin moves.
if (llvm::all_of(Filtered, [&](Value *Arg) { return Arg == AllSameValue; })) {
+ // Can't fold phi(undef, X) -> X unless X can't be poison (thus X is undef
+ // in the worst case).
+ if (HasUndef && !isGuaranteedNotToBePoison(AllSameValue, AC, nullptr, DT))
+ return E;
+
// In LLVM's non-standard representation of phi nodes, it's possible to have
// phi nodes with cycles (IE dependent on other phis that are .... dependent
// on the original phi node), especially in weird CFG's where some arguments
@@ -1764,8 +1770,8 @@ NewGVN::performSymbolicPHIEvaluation(ArrayRef<ValPair> PHIOps,
// infinite loops during evaluation. We work around this by not trying to
// really evaluate them independently, but instead using a variable
// expression to say if one is equivalent to the other.
- // We also special case undef, so that if we have an undef, we can't use the
- // common value unless it dominates the phi block.
+ // We also special case undef/poison, so that if we have an undef, we can't
+ // use the common value unless it dominates the phi block.
if (HasPoison || HasUndef) {
// If we have undef and at least one other value, this is really a
// multivalued phi, and we need to know if it's cycle free in order to
@@ -2853,14 +2859,14 @@ NewGVN::makePossiblePHIOfOps(Instruction *I,
}
// The algorithm initially places the values of the routine in the TOP
-// congruence class. The leader of TOP is the undetermined value `undef`.
+// congruence class. The leader of TOP is the undetermined value `poison`.
// When the algorithm has finished, values still in TOP are unreachable.
void NewGVN::initializeCongruenceClasses(Function &F) {
NextCongruenceNum = 0;
// Note that even though we use the live on entry def as a representative
// MemoryAccess, it is *not* the same as the actual live on entry def. We
- // have no real equivalemnt to undef for MemoryAccesses, and so we really
+ // have no real equivalent to poison for MemoryAccesses, and so we really
// should be checking whether the MemoryAccess is top if we want to know if it
// is equivalent to everything. Otherwise, what this really signifies is that
// the access "it reaches all the way back to the beginning of the function"
@@ -3031,7 +3037,7 @@ void NewGVN::valueNumberMemoryPhi(MemoryPhi *MP) {
!isMemoryAccessTOP(cast<MemoryAccess>(U)) &&
ReachableEdges.count({MP->getIncomingBlock(U), PHIBlock});
});
- // If all that is left is nothing, our memoryphi is undef. We keep it as
+ // If all that is left is nothing, our memoryphi is poison. We keep it as
// InitialClass. Note: The only case this should happen is if we have at
// least one self-argument.
if (Filtered.begin() == Filtered.end()) {
diff --git a/llvm/test/Transforms/NewGVN/completeness.ll b/llvm/test/Transforms/NewGVN/completeness.ll
index 3131e84fe28d0..73cfb6c497659 100644
--- a/llvm/test/Transforms/NewGVN/completeness.ll
+++ b/llvm/test/Transforms/NewGVN/completeness.ll
@@ -505,17 +505,16 @@ declare i32* @wombat()
;; change in the verifier, as it goes from a constant value to a
;; phi of [true, false]
-define void @test12() {
+define void @test12(i32* %p) {
; CHECK-LABEL: @test12(
; CHECK-NEXT: bb:
-; CHECK-NEXT: [[TMP:%.*]] = load i32, i32* null
+; CHECK-NEXT: [[TMP:%.*]] = load i32, i32* %p
; CHECK-NEXT: [[TMP1:%.*]] = icmp sgt i32 [[TMP]], 0
; CHECK-NEXT: br i1 [[TMP1]], label [[BB2:%.*]], label [[BB8:%.*]]
; CHECK: bb2:
; CHECK-NEXT: br label [[BB3:%.*]]
; CHECK: bb3:
-; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i1 [ true, [[BB2]] ], [ false, [[BB7:%.*]] ]
-; CHECK-NEXT: br i1 [[PHIOFOPS]], label [[BB6:%.*]], label [[BB7]]
+; CHECK-NEXT: br i1 true, label [[BB6:%.*]], label [[BB7]]
; CHECK: bb6:
; CHECK-NEXT: br label [[BB7]]
; CHECK: bb7:
@@ -524,7 +523,7 @@ define void @test12() {
; CHECK-NEXT: ret void
;
bb:
- %tmp = load i32, i32* null
+ %tmp = load i32, i32* %p
%tmp1 = icmp sgt i32 %tmp, 0
br i1 %tmp1, label %bb2, label %bb8
@@ -532,7 +531,7 @@ bb2: ; preds = %bb
br label %bb3
bb3: ; preds = %bb7, %bb2
- %tmp4 = phi i32 [ %tmp, %bb2 ], [ undef, %bb7 ]
+ %tmp4 = phi i32 [ %tmp, %bb2 ], [ poison, %bb7 ]
%tmp5 = icmp sgt i32 %tmp4, 0
br i1 %tmp5, label %bb6, label %bb7
diff --git a/llvm/test/Transforms/NewGVN/phi-edge-handling.ll b/llvm/test/Transforms/NewGVN/phi-edge-handling.ll
index fc31a8d78b568..8dfac7942210c 100644
--- a/llvm/test/Transforms/NewGVN/phi-edge-handling.ll
+++ b/llvm/test/Transforms/NewGVN/phi-edge-handling.ll
@@ -119,6 +119,27 @@ define i8 @value_undef(i1 %cond, i8 %v) {
; CHECK: B:
; CHECK-NEXT: br label [[EXIT]]
; CHECK: EXIT:
+; CHECK-NEXT: [[R:%.*]] = phi i8 [ undef, [[A]] ], [ [[V:%.*]], [[B]] ]
+; CHECK-NEXT: ret i8 [[R]]
+;
+ br i1 %cond, label %A, label %B
+A:
+ br label %EXIT
+B:
+ br label %EXIT
+EXIT:
+ %r = phi i8 [undef, %A], [%v, %B]
+ ret i8 %r
+}
+
+define i8 @value_undef_noundef(i1 %cond, i8 noundef %v) {
+; CHECK-LABEL: @value_undef_noundef(
+; CHECK-NEXT: br i1 [[COND:%.*]], label [[A:%.*]], label [[B:%.*]]
+; CHECK: A:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: B:
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: EXIT:
; CHECK-NEXT: ret i8 [[V:%.*]]
;
br i1 %cond, label %A, label %B
diff --git a/llvm/test/Transforms/NewGVN/pr31682.ll b/llvm/test/Transforms/NewGVN/pr31682.ll
index b1638c30b107f..03ee9f3f7d9fe 100644
--- a/llvm/test/Transforms/NewGVN/pr31682.ll
+++ b/llvm/test/Transforms/NewGVN/pr31682.ll
@@ -24,7 +24,7 @@ bb:
br label %bb2
bb2: ; preds = %bb2, %bb
- %tmp3 = phi %struct.foo* [ undef, %bb ], [ %tmp6, %bb2 ]
+ %tmp3 = phi %struct.foo* [ poison, %bb ], [ %tmp6, %bb2 ]
%tmp4 = getelementptr %struct.foo, %struct.foo* %tmp3, i64 0, i32 1
%tmp5 = load i32, i32* %tmp4
%tmp6 = load %struct.foo*, %struct.foo** @global
diff --git a/llvm/test/Transforms/NewGVN/pr32403.ll b/llvm/test/Transforms/NewGVN/pr32403.ll
index 89b3afffb2e48..90a2907d02ca7 100644
--- a/llvm/test/Transforms/NewGVN/pr32403.ll
+++ b/llvm/test/Transforms/NewGVN/pr32403.ll
@@ -50,7 +50,7 @@ if.then17.i: ; preds = %for.body8.i
br label %for.inc24.i
for.inc24.i: ; preds = %if.then17.i, %for.body8.i
- %nIdx.1.i = phi i32 [ undef, %if.then17.i ], [ %nIdx.052.i, %for.body8.i ]
+ %nIdx.1.i = phi i32 [ poison, %if.then17.i ], [ %nIdx.052.i, %for.body8.i ]
br label %for.body8.i
if.else58: ; preds = %for.body
diff --git a/llvm/test/Transforms/NewGVN/pr32838.ll b/llvm/test/Transforms/NewGVN/pr32838.ll
index 788ee704ec47e..594f8d2ba4481 100644
--- a/llvm/test/Transforms/NewGVN/pr32838.ll
+++ b/llvm/test/Transforms/NewGVN/pr32838.ll
@@ -3,7 +3,7 @@
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.12.0"
;; Ensure we don't infinite loop when all phi arguments are really unreachable or self-defined
-define void @fn1(i64 %arg) {
+define void @fn1(i64 noundef %arg) {
; CHECK-LABEL: @fn1(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 undef, label [[IF_THEN:%.*]], label [[COND_TRUE:%.*]]
@@ -47,7 +47,7 @@ cond.true:
temp:
ret void
}
-define void @fn2(i64 %arg) {
+define void @fn2(i64 noundef %arg) {
; CHECK-LABEL: @fn2(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 undef, label [[IF_THEN:%.*]], label [[COND_TRUE:%.*]]
diff --git a/llvm/test/Transforms/NewGVN/pr34135.ll b/llvm/test/Transforms/NewGVN/pr34135.ll
index 4b4aae9304f9a..0b6f9a24b1d79 100644
--- a/llvm/test/Transforms/NewGVN/pr34135.ll
+++ b/llvm/test/Transforms/NewGVN/pr34135.ll
@@ -3,7 +3,7 @@
;; Make sure we don't incorrectly use predicateinfo to simplify phi of ops cases
source_filename = "pr34135.ll"
-define void @snork(i32 %arg) {
+define void @snork(i32 noundef %arg) {
; CHECK-LABEL: @snork(
; CHECK-NEXT: bb:
; CHECK-NEXT: [[TMP:%.*]] = sext i32 [[ARG:%.*]] to i64
More information about the llvm-commits
mailing list