diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp --- 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 @@ 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 @@ ++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 @@ // 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 @@ } // 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 @@ !isMemoryAccessTOP(cast(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 --- a/llvm/test/Transforms/NewGVN/completeness.ll +++ b/llvm/test/Transforms/NewGVN/completeness.ll @@ -505,17 +505,16 @@ ;; 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 @@ ; 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 @@ 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 --- a/llvm/test/Transforms/NewGVN/phi-edge-handling.ll +++ b/llvm/test/Transforms/NewGVN/phi-edge-handling.ll @@ -119,6 +119,27 @@ ; 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 --- a/llvm/test/Transforms/NewGVN/pr31682.ll +++ b/llvm/test/Transforms/NewGVN/pr31682.ll @@ -24,7 +24,7 @@ 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 --- a/llvm/test/Transforms/NewGVN/pr32403.ll +++ b/llvm/test/Transforms/NewGVN/pr32403.ll @@ -50,7 +50,7 @@ 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 --- 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 @@ 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 --- 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