Skip to content

Commit

Permalink
[NewPM][CGSCC] Handle newly added functions in updateCGAndAnalysisMan…
Browse files Browse the repository at this point in the history 
…agerForPass

This seems to fit the CGSCC updates model better than calling
addNewFunctionInto{Ref,}SCC() on newly created/outlined functions.
Now addNewFunctionInto{Ref,}SCC() are no longer necessary.

However, this doesn't work on newly outlined functions that aren't
referenced by the original function. e.g. if a() was outlined into b()
and c(), but c() is only referenced by b() and not by a(), this will
trigger an assert.

This also fixes an issue I was seeing with newly created functions not
having passes run on them.

Ran check-llvm with expensive checks.

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D87798
  • Loading branch information
@aeubanks
aeubanks committed Sep 23, 2020
1 parent 6700b9d commit 6b1ce83
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 146 deletions.
18 changes: 4 additions & 14 deletions llvm/include/llvm/Analysis/LazyCallGraph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,10 @@ class LazyCallGraph {
/// remain active and reachable.
bool isLibFunction(Function &F) const { return LibFunctions.count(&F); }

/// Helper to initialize a new node created outside of creating SCCs and add
/// it to the NodeMap. e.g. when a function is outlined.
Node &initNode(Node &N, LazyCallGraph::SCC &C);

///@{
/// \name Pre-SCC Mutation API
///
Expand Down Expand Up @@ -1058,13 +1062,6 @@ class LazyCallGraph {
/// fully visited by the DFS prior to calling this routine.
void removeDeadFunction(Function &F);

/// Introduce a node for the function \p NewF in the SCC \p C.
void addNewFunctionIntoSCC(Function &NewF, SCC &C);

/// Introduce a node for the function \p NewF, as a single node in a
/// new SCC, in the RefSCC \p RC.
void addNewFunctionIntoRefSCC(Function &NewF, RefSCC &RC);

///@}

///@{
Expand Down Expand Up @@ -1171,13 +1168,6 @@ class LazyCallGraph {
/// Helper to update pointers back to the graph object during moves.
void updateGraphPtrs();

/// Helper to insert a new function, add it to the NodeMap, and populate its
/// node.
Node &createNode(Function &F);

/// Helper to add the given Node \p N to the SCCMap, mapped to the SCC \p C.
void addNodeToSCC(SCC &C, Node &N);

/// Allocates an SCC and constructs it using the graph allocator.
///
/// The arguments are forwarded to the constructor.
Expand Down
32 changes: 22 additions & 10 deletions llvm/lib/Analysis/CGSCCPassManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
SmallSetVector<Node *, 4> DemotedCallTargets;
SmallSetVector<Node *, 4> NewCallEdges;
SmallSetVector<Node *, 4> NewRefEdges;
SmallSetVector<Node *, 4> NewNodes;

// First walk the function and handle all called functions. We do this first
// because if there is a single call edge, whether there are ref edges is
Expand All @@ -467,19 +468,23 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
if (auto *CB = dyn_cast<CallBase>(&I))
if (Function *Callee = CB->getCalledFunction())
if (Visited.insert(Callee).second && !Callee->isDeclaration()) {
Node &CalleeN = *G.lookup(*Callee);
Edge *E = N->lookup(CalleeN);
Node *CalleeN = G.lookup(*Callee);
if (!CalleeN) {
CalleeN = &G.get(*Callee);
NewNodes.insert(CalleeN);
}
Edge *E = N->lookup(*CalleeN);
assert((E || !FunctionPass) &&
"No function transformations should introduce *new* "
"call edges! Any new calls should be modeled as "
"promoted existing ref edges!");
bool Inserted = RetainedEdges.insert(&CalleeN).second;
bool Inserted = RetainedEdges.insert(CalleeN).second;
(void)Inserted;
assert(Inserted && "We should never visit a function twice.");
if (!E)
NewCallEdges.insert(&CalleeN);
NewCallEdges.insert(CalleeN);
else if (!E->isCall())
PromotedRefTargets.insert(&CalleeN);
PromotedRefTargets.insert(CalleeN);
}

// Now walk all references.
Expand All @@ -490,22 +495,29 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
Worklist.push_back(C);

auto VisitRef = [&](Function &Referee) {
Node &RefereeN = *G.lookup(Referee);
Edge *E = N->lookup(RefereeN);
Node *RefereeN = G.lookup(Referee);
if (!RefereeN) {
RefereeN = &G.get(Referee);
NewNodes.insert(RefereeN);
}
Edge *E = N->lookup(*RefereeN);
assert((E || !FunctionPass) &&
"No function transformations should introduce *new* ref "
"edges! Any new ref edges would require IPO which "
"function passes aren't allowed to do!");
bool Inserted = RetainedEdges.insert(&RefereeN).second;
bool Inserted = RetainedEdges.insert(RefereeN).second;
(void)Inserted;
assert(Inserted && "We should never visit a function twice.");
if (!E)
NewRefEdges.insert(&RefereeN);
NewRefEdges.insert(RefereeN);
else if (E->isCall())
DemotedCallTargets.insert(&RefereeN);
DemotedCallTargets.insert(RefereeN);
};
LazyCallGraph::visitReferences(Worklist, Visited, VisitRef);

for (Node *NewNode : NewNodes)
G.initNode(*NewNode, *C);

// Handle new ref edges.
for (Node *RefTarget : NewRefEdges) {
SCC &TargetC = *G.lookupSCC(*RefTarget);
Expand Down
25 changes: 3 additions & 22 deletions llvm/lib/Analysis/LazyCallGraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1565,21 +1565,6 @@ void LazyCallGraph::removeDeadFunction(Function &F) {
// allocators.
}

void LazyCallGraph::addNewFunctionIntoSCC(Function &NewF, SCC &C) {
addNodeToSCC(C, createNode(NewF));
}

void LazyCallGraph::addNewFunctionIntoRefSCC(Function &NewF, RefSCC &RC) {
Node &N = createNode(NewF);

auto *C = createSCC(RC, SmallVector<Node *, 1>());
addNodeToSCC(*C, N);

auto Index = RC.SCCIndices.size();
RC.SCCIndices[C] = Index;
RC.SCCs.push_back(C);
}

LazyCallGraph::Node &LazyCallGraph::insertInto(Function &F, Node *&MappedN) {
return *new (MappedN = BPA.Allocate()) Node(*this, F);
}
Expand All @@ -1594,17 +1579,13 @@ void LazyCallGraph::updateGraphPtrs() {
RC->G = this;
}

LazyCallGraph::Node &LazyCallGraph::createNode(Function &F) {
Node &N = get(F);
NodeMap[&F] = &N;
LazyCallGraph::Node &LazyCallGraph::initNode(Node &N, LazyCallGraph::SCC &C) {
NodeMap[&N.getFunction()] = &N;
N.DFSNumber = N.LowLink = -1;
N.populate();
return N;
}

void LazyCallGraph::addNodeToSCC(LazyCallGraph::SCC &C, Node &N) {
C.Nodes.push_back(&N);
SCCMap[&N] = &C;
return N;
}

template <typename RootsT, typename GetBeginT, typename GetEndT,
Expand Down
7 changes: 0 additions & 7 deletions llvm/lib/Transforms/Coroutines/CoroSplit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1484,13 +1484,6 @@ static void updateCallGraphAfterCoroutineSplit(

postSplitCleanup(N.getFunction());

// To insert the newly created coroutine funclets 'f.resume', 'f.destroy', and
// 'f.cleanup' into the same SCC as the coroutine 'f' they were outlined from,
// we make use of the CallGraphUpdater class, which can modify the internal
// state of the LazyCallGraph.
for (Function *Clone : Clones)
CG.addNewFunctionIntoRefSCC(*Clone, C.getOuterRefSCC());

// We've inserted instructions into coroutine 'f' that reference the three new
// coroutine funclets. We must now update the call graph so that reference
// edges between 'f' and its funclets are added to it. LazyCallGraph only
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ void CallGraphUpdater::reanalyzeFunction(Function &Fn) {
void CallGraphUpdater::registerOutlinedFunction(Function &NewFn) {
if (CG)
CG->addToCallGraph(&NewFn);
else if (LCG)
LCG->addNewFunctionIntoSCC(NewFn, *SCC);
}

void CallGraphUpdater::removeFunction(Function &DeadFn) {
Expand Down
33 changes: 25 additions & 8 deletions llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1743,7 +1743,6 @@ TEST_F(CGSCCPassManagerTest, TestInsertionOfNewRefSCC) {
// single node in a new SCC, into the call graph. As a result
// the call graph is composed of a single RefSCC with two SCCs:
// [(f), (g)].
CG.addNewFunctionIntoRefSCC(*G, C.getOuterRefSCC());

// "Demote" the 'f -> f' call egde to a ref edge.
// 1. Erase the call edge from 'f' to 'f'.
Expand Down Expand Up @@ -1785,6 +1784,25 @@ TEST_F(CGSCCPassManagerTest, TestInsertionOfNewRefSCCMutuallyRecursive) {
if (F.getName() != "f")
continue;

// Create mutually recursive functions (call) 'g1' and 'g2'.
auto *G1 = Function::Create(F.getFunctionType(), F.getLinkage(),
F.getAddressSpace(), "g1", F.getParent());
auto *G2 = Function::Create(F.getFunctionType(), F.getLinkage(),
F.getAddressSpace(), "g2", F.getParent());
BasicBlock *G1BB =
BasicBlock::Create(F.getParent()->getContext(), "entry", G1);
BasicBlock *G2BB =
BasicBlock::Create(F.getParent()->getContext(), "entry", G2);
(void)CallInst::Create(G1, {}, "", G1BB);
(void)ReturnInst::Create(G1->getContext(), G1BB);
(void)CallInst::Create(G2, {}, "", G1BB);
(void)ReturnInst::Create(G2->getContext(), G2BB);

// Add 'f -> g1' call edge.
(void)CallInst::Create(G1, {}, "", &F.getEntryBlock().front());
// Add 'f -> g2' call edge.
(void)CallInst::Create(G2, {}, "", &F.getEntryBlock().front());

// Create mutually recursive functions (ref only) 'h1' and 'h2'.
auto *H1 = Function::Create(F.getFunctionType(), F.getLinkage(),
F.getAddressSpace(), "h1", F.getParent());
Expand All @@ -1803,16 +1821,15 @@ TEST_F(CGSCCPassManagerTest, TestInsertionOfNewRefSCCMutuallyRecursive) {

// Add 'f -> h1' ref edge.
(void)CastInst::CreatePointerCast(H1, Type::getInt8PtrTy(F.getContext()),
"h.ref", &F.getEntryBlock().front());

CG.addNewFunctionIntoRefSCC(*H1, C.getOuterRefSCC());
CG.addNewFunctionIntoRefSCC(*H2, C.getOuterRefSCC());
"h1.ref", &F.getEntryBlock().front());
// Add 'f -> h2' ref edge.
(void)CastInst::CreatePointerCast(H2, Type::getInt8PtrTy(F.getContext()),
"h2.ref", &F.getEntryBlock().front());

ASSERT_NO_FATAL_FAILURE(
updateCGAndAnalysisManagerForCGSCCPass(CG, C, N, AM, UR, FAM))
<< "Updating the call graph with a demoted, self-referential "
"call edge 'f -> f', a newly inserted ref edge 'f -> g', and "
"mutually recursive h1 <-> h2 caused a fatal failure";
<< "Updating the call graph with mutually recursive g1 <-> g2, h1 "
"<-> h2 caused a fatal failure";
}
}));

Expand Down
83 changes: 0 additions & 83 deletions llvm/unittests/Analysis/LazyCallGraphTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,47 +451,6 @@ TEST(LazyCallGraphTest, BasicGraphMutation) {
EXPECT_EQ(0, std::distance(B->begin(), B->end()));
}

TEST(LazyCallGraphTest, BasicGraphMutationOutlining) {
LLVMContext Context;
std::unique_ptr<Module> M = parseAssembly(Context, "define void @a() {\n"
"entry:\n"
" call void @b()\n"
" call void @c()\n"
" ret void\n"
"}\n"
"define void @b() {\n"
"entry:\n"
" ret void\n"
"}\n"
"define void @c() {\n"
"entry:\n"
" ret void\n"
"}\n");
LazyCallGraph CG = buildCG(*M);

LazyCallGraph::Node &A = CG.get(lookupFunction(*M, "a"));
LazyCallGraph::Node &B = CG.get(lookupFunction(*M, "b"));
LazyCallGraph::Node &C = CG.get(lookupFunction(*M, "c"));
A.populate();
B.populate();
C.populate();
CG.buildRefSCCs();

// Add a new function that is called from @b and verify it is in the same SCC.
Function &BFn = B.getFunction();
Function *NewFn =
Function::Create(BFn.getFunctionType(), BFn.getLinkage(), "NewFn", *M);
auto IP = BFn.getEntryBlock().getFirstInsertionPt();
CallInst::Create(NewFn, "", &*IP);
CG.addNewFunctionIntoSCC(*NewFn, *CG.lookupSCC(B));

EXPECT_EQ(CG.lookupSCC(A)->size(), 1);
EXPECT_EQ(CG.lookupSCC(B)->size(), 2);
EXPECT_EQ(CG.lookupSCC(C)->size(), 1);
EXPECT_EQ(CG.lookupSCC(*CG.lookup(*NewFn))->size(), 2);
EXPECT_EQ(CG.lookupSCC(*CG.lookup(*NewFn))->size(), CG.lookupSCC(B)->size());
}

TEST(LazyCallGraphTest, InnerSCCFormation) {
LLVMContext Context;
std::unique_ptr<Module> M = parseAssembly(Context, DiamondOfTriangles);
Expand Down Expand Up @@ -2211,46 +2170,4 @@ TEST(LazyCallGraphTest, RemoveFunctionWithSpurriousRef) {
EXPECT_EQ(&RC2, &*I++);
EXPECT_EQ(CG.postorder_ref_scc_end(), I);
}

TEST(LazyCallGraphTest, AddNewFunctionIntoRefSCC) {
LLVMContext Context;
// Build and populate a graph composed of a single, self-referential node.
std::unique_ptr<Module> M = parseAssembly(Context, "define void @f() {\n"
"entry:\n"
" call void @f()\n"
" ret void\n"
"}\n");
LazyCallGraph CG = buildCG(*M);
CG.buildRefSCCs();

// At this point 'f' is in the call graph.
auto &F = lookupFunction(*M, "f");
LazyCallGraph::Node *FN = CG.lookup(F);
EXPECT_NE(FN, nullptr);

// And it has an SCC, of course.
auto *FSCC = CG.lookupSCC(*FN);
EXPECT_NE(FSCC, nullptr);

// Now, create a new function 'g'.
auto *G = Function::Create(F.getFunctionType(), F.getLinkage(),
F.getAddressSpace(), "g", F.getParent());
BasicBlock::Create(F.getParent()->getContext(), "entry", G);

// Instruct the LazyCallGraph to create a new node for 'g', within the same
// RefSCC as 'f', but in a separate SCC.
CG.addNewFunctionIntoRefSCC(*G, FSCC->getOuterRefSCC());

// 'g' should now be in the call graph.
LazyCallGraph::Node *GN = CG.lookup(*G);
EXPECT_NE(GN, nullptr);
// 'g' should have an SCC, composed of the singular node 'g'.
// ('f' should not be included in the 'g' SCC.)
LazyCallGraph::SCC *GSCC = CG.lookupSCC(*GN);
EXPECT_NE(GSCC, nullptr);
EXPECT_EQ(GSCC->size(), 1);
EXPECT_NE(GSCC, FSCC);
// 'g' and 'f' should be part of the same RefSCC.
EXPECT_EQ(&GSCC->getOuterRefSCC(), &FSCC->getOuterRefSCC());
}
}

0 comments on commit 6b1ce83

Please sign in to comment.