[llvm] r303393 - [NewGVN] Break infinite recursion in singleReachablePHIPath().
Davide Italiano via llvm-commits
llvm-commits at lists.llvm.org
Thu May 18 16:22:45 PDT 2017
Author: davide
Date: Thu May 18 18:22:44 2017
New Revision: 303393
URL: http://llvm.org/viewvc/llvm-project?rev=303393&view=rev
Log:
[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.
Added:
llvm/trunk/test/Transforms/NewGVN/pr33014.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=303393&r1=303392&r2=303393&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Thu May 18 18:22:44 2017
@@ -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(Inst
// 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(cons
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() co
"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.
Added: llvm/trunk/test/Transforms/NewGVN/pr33014.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/pr33014.ll?rev=303393&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/pr33014.ll (added)
+++ llvm/trunk/test/Transforms/NewGVN/pr33014.ll Thu May 18 18:22:44 2017
@@ -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
+
+ at 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
+}
More information about the llvm-commits
mailing list