[NewGVN] Break infinite recursion in singleReachablePHIPath().

We can have cycles between PHIs and this causes singleReachablePhi()
to call itself indefintely (until we run out of stack). The proper
solution would be that of computing SCCs, but it's not worth for
now, so just keep a visited set and give up when we find a cycle.
Thanks to Dan for the discussion/help with this.

Fixes PR33014.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@303393 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Davide Italiano 2017-05-18 23:22:44 +00:00
parent bde49e3081
commit ed2b8bd9f4
2 changed files with 74 additions and 6 deletions

View File

@ -643,7 +643,8 @@ private:
void verifyMemoryCongruency() const;
void verifyIterationSettled(Function &F);
void verifyStoreExpressions() const;
bool singleReachablePHIPath(const MemoryAccess *, const MemoryAccess *) const;
bool singleReachablePHIPath(SmallPtrSet<const MemoryAccess *, 8> &,
const MemoryAccess *, const MemoryAccess *) const;
BasicBlock *getBlockForValue(Value *V) const;
void deleteExpression(const Expression *E) const;
unsigned InstrToDFSNum(const Value *V) const {
@ -2460,13 +2461,23 @@ void NewGVN::valueNumberInstruction(Instruction *I) {
// Check if there is a path, using single or equal argument phi nodes, from
// First to Second.
bool NewGVN::singleReachablePHIPath(const MemoryAccess *First,
const MemoryAccess *Second) const {
bool NewGVN::singleReachablePHIPath(
SmallPtrSet<const MemoryAccess *, 8> &Visited, const MemoryAccess *First,
const MemoryAccess *Second) const {
if (First == Second)
return true;
if (MSSA->isLiveOnEntryDef(First))
return false;
// This is not perfect, but as we're just verifying here, we can live with
// the loss of precision. The real solution would be that of doing strongly
// connected component finding in this routine, and it's probably not worth
// the complexity for the time being. So, we just keep a set of visited
// MemoryAccess and return true when we hit a cycle.
if (Visited.count(First))
return true;
Visited.insert(First);
const auto *EndDef = First;
for (auto *ChainDef : optimized_def_chain(First)) {
if (ChainDef == Second)
@ -2489,7 +2500,8 @@ bool NewGVN::singleReachablePHIPath(const MemoryAccess *First,
Okay =
std::equal(OperandList.begin(), OperandList.end(), OperandList.begin());
if (Okay)
return singleReachablePHIPath(cast<MemoryAccess>(OperandList[0]), Second);
return singleReachablePHIPath(Visited, cast<MemoryAccess>(OperandList[0]),
Second);
return false;
}
@ -2556,13 +2568,15 @@ void NewGVN::verifyMemoryCongruency() const {
"Memory not unreachable but ended up in TOP");
if (auto *FirstMUD = dyn_cast<MemoryUseOrDef>(KV.first)) {
auto *SecondMUD = dyn_cast<MemoryUseOrDef>(KV.second->getMemoryLeader());
if (FirstMUD && SecondMUD)
assert((singleReachablePHIPath(FirstMUD, SecondMUD) ||
if (FirstMUD && SecondMUD) {
SmallPtrSet<const MemoryAccess *, 8> VisitedMAS;
assert((singleReachablePHIPath(VisitedMAS, FirstMUD, SecondMUD) ||
ValueToClass.lookup(FirstMUD->getMemoryInst()) ==
ValueToClass.lookup(SecondMUD->getMemoryInst())) &&
"The instructions for these memory operations should have "
"been in the same congruence class or reachable through"
"a single argument phi");
}
} else if (auto *FirstMP = dyn_cast<MemoryPhi>(KV.first)) {
// We can only sanely verify that MemoryDefs in the operand list all have
// the same class.

View File

@ -0,0 +1,54 @@
; Make sure we don't end up in an infinite recursion in singleReachablePHIPath().
; REQUIRES: asserts
; RUN: opt -newgvn -S %s | FileCheck %s
@c = external global i64, align 8
; CHECK-LABEL: define void @tinkywinky() {
; CHECK: entry:
; CHECK-NEXT: br i1 undef, label %l2, label %if.then
; CHECK: if.then: ; preds = %entry
; CHECK-NEXT: br label %for.body
; CHECK: ph: ; preds = %back, %ontrue
; CHECK-NEXT: br label %for.body
; CHECK: for.body: ; preds = %ph, %if.then
; CHECK-NEXT: br i1 undef, label %ontrue, label %onfalse
; CHECK: onfalse: ; preds = %for.body
; CHECK-NEXT: %patatino = load i64, i64* @c
; CHECK-NEXT: ret void
; CHECK: ontrue: ; preds = %for.body
; CHECK-NEXT: %dipsy = load i64, i64* @c
; CHECK-NEXT: br label %ph
; CHECK: back: ; preds = %l2
; CHECK-NEXT: store i8 undef, i8* null
; CHECK-NEXT: br label %ph
; CHECK: end: ; preds = %l2
; CHECK-NEXT: ret void
; CHECK: l2: ; preds = %entry
; CHECK-NEXT: br i1 false, label %back, label %end
; CHECK-NEXT: }
define void @tinkywinky() {
entry:
br i1 undef, label %l2, label %if.then
if.then:
br label %for.body
ph:
br label %for.body
for.body:
br i1 undef, label %ontrue, label %onfalse
onfalse:
%patatino = load i64, i64* @c
store i64 %patatino, i64* @c
ret void
ontrue:
%dipsy = load i64, i64* @c
store i64 %dipsy, i64* @c
br label %ph
back:
br label %ph
end:
ret void
l2:
br i1 false, label %back, label %end
}