[llvm] 64d99a1 - [CallGraph] Update callback call sites in RefreshCallGraph
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 14 20:36:24 PDT 2020
Author: Johannes Doerfert
Date: 2020-07-14T22:33:57-05:00
New Revision: 64d99a1d0476b8a451c3b36d900e84bc5707c061
URL: https://github.com/llvm/llvm-project/commit/64d99a1d0476b8a451c3b36d900e84bc5707c061
DIFF: https://github.com/llvm/llvm-project/commit/64d99a1d0476b8a451c3b36d900e84bc5707c061.diff
LOG: [CallGraph] Update callback call sites in RefreshCallGraph
Since D82572, we keep "reference" edges for callback call sites. While
not strictly necessary they can improve the traversal order. However, we
did not update them properly in case a pass removed the callback call
site which caused a verification error (PR46687). With this patch we
update these reference edges properly during the invocation of
`CallGraphSCCPass::RefreshCallGraph` in non-checking mode.
Reviewed By: sdmitriev
Differential Revision: https://reviews.llvm.org/D83718
Added:
llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
Modified:
llvm/lib/Analysis/CallGraphSCCPass.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/CallGraphSCCPass.cpp b/llvm/lib/Analysis/CallGraphSCCPass.cpp
index fc65936024af..91f8029cc326 100644
--- a/llvm/lib/Analysis/CallGraphSCCPass.cpp
+++ b/llvm/lib/Analysis/CallGraphSCCPass.cpp
@@ -19,6 +19,7 @@
#include "llvm/ADT/SCCIterator.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/CallGraph.h"
+#include "llvm/IR/AbstractCallSite.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRPrintingPasses.h"
#include "llvm/IR/Intrinsics.h"
@@ -225,11 +226,35 @@ bool CGPassManager::RefreshCallGraph(const CallGraphSCC &CurSCC, CallGraph &CG,
// invalidated and removed.
unsigned NumDirectRemoved = 0, NumIndirectRemoved = 0;
+ CallGraphNode::iterator CGNEnd = CGN->end();
+
+ auto RemoveAndCheckForDone = [&](CallGraphNode::iterator I) {
+ // Just remove the edge from the set of callees, keep track of whether
+ // I points to the last element of the vector.
+ bool WasLast = I + 1 == CGNEnd;
+ CGN->removeCallEdge(I);
+
+ // If I pointed to the last element of the vector, we have to bail out:
+ // iterator checking rejects comparisons of the resultant pointer with
+ // end.
+ if (WasLast)
+ return true;
+
+ CGNEnd = CGN->end();
+ return false;
+ };
+
// Get the set of call sites currently in the function.
- for (CallGraphNode::iterator I = CGN->begin(), E = CGN->end(); I != E; ) {
- // Skip "reference" call records that do not have call instruction.
+ for (CallGraphNode::iterator I = CGN->begin(); I != CGNEnd;) {
+ // Delete "reference" call records that do not have call instruction. We
+ // reinsert them as needed later. However, keep them in checking mode.
if (!I->first) {
- ++I;
+ if (CheckingMode) {
+ ++I;
+ continue;
+ }
+ if (RemoveAndCheckForDone(I))
+ break;
continue;
}
@@ -258,17 +283,8 @@ bool CGPassManager::RefreshCallGraph(const CallGraphSCC &CurSCC, CallGraph &CG,
else
++NumDirectRemoved;
- // Just remove the edge from the set of callees, keep track of whether
- // I points to the last element of the vector.
- bool WasLast = I + 1 == E;
- CGN->removeCallEdge(I);
-
- // If I pointed to the last element of the vector, we have to bail out:
- // iterator checking rejects comparisons of the resultant pointer with
- // end.
- if (WasLast)
+ if (RemoveAndCheckForDone(I))
break;
- E = CGN->end();
continue;
}
@@ -296,6 +312,15 @@ bool CGPassManager::RefreshCallGraph(const CallGraphSCC &CurSCC, CallGraph &CG,
if (Callee && Callee->isIntrinsic())
continue;
+ // If we are not in checking mode, insert potential callback calls as
+ // references. This is not a requirement but helps to iterate over the
+ // functions in the right order.
+ if (!CheckingMode) {
+ forEachCallbackFunction(*Call, [&](Function *CB) {
+ CGN->addCalledFunction(nullptr, CG.getOrInsertFunction(CB));
+ });
+ }
+
// If this call site already existed in the callgraph, just verify it
// matches up to expectations and remove it from Calls.
DenseMap<Value *, CallGraphNode *>::iterator ExistingIt =
diff --git a/llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll b/llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
new file mode 100644
index 000000000000..168d5bf46b3c
--- /dev/null
+++ b/llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
@@ -0,0 +1,89 @@
+; RUN: opt < %s -instcombine -attributor-cgscc -print-callgraph -disable-output -verify 2>&1 | FileCheck %s
+
+; CHECK: Call graph node <<null function>><<{{.*}}>> #uses=0
+; CHECK: CS<None> calls function 'dead_fork_call'
+; CHECK: CS<None> calls function 'd'
+; CHECK: CS<None> calls function '__kmpc_fork_call'
+; CHECK: CS<None> calls function 'live_fork_call'
+; CHECK: CS<None> calls function '.omp_outlined..1'
+;
+; CHECK: Call graph node for function: '.omp_outlined..1'<<{{.*}}>> #uses=3
+; CHECK: CS<{{.*}}> calls function 'd'
+;
+; CHECK: Call graph node for function: '__kmpc_fork_call'<<{{.*}}>> #uses=3
+; CHECK: CS<None> calls external node
+;
+; CHECK: Call graph node for function: 'd'<<{{.*}}>> #uses=2
+; CHECK: CS<None> calls external node
+;
+; CHECK: Call graph node for function: 'dead_fork_call'<<{{.*}}>> #uses=1
+;
+; CHECK: Call graph node for function: 'dead_fork_call2'<<{{.*}}>> #uses=0
+; CHECK: CS<{{.*}}> calls function '__kmpc_fork_call'
+; CHECK: CS<None> calls function '.omp_outlined..1'
+;
+; CHECK: Call graph node for function: 'live_fork_call'<<{{.*}}>> #uses=1
+; CHECK: CS<{{.*}}> calls function '__kmpc_fork_call'
+; CHECK: CS<None> calls function '.omp_outlined..1'
+
+
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
+
+ at .str = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
+ at 0 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str, i32 0, i32 0) }, align 8
+
+define dso_local void @dead_fork_call() {
+entry:
+ br i1 true, label %if.then, label %if.else
+
+if.then: ; preds = %entry
+ br label %if.end
+
+if.else: ; preds = %entry
+ call void @dead_fork_call2()
+ call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @0, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @.omp_outlined..0 to void (i32*, i32*, ...)*))
+ br label %if.end
+
+if.end: ; preds = %if.else, %if.then
+ ret void
+}
+
+define internal void @dead_fork_call2() {
+entry:
+ call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @0, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @.omp_outlined..1 to void (i32*, i32*, ...)*))
+ ret void
+}
+
+define internal void @.omp_outlined..0(i32* noalias %.global_tid., i32* noalias %.bound_tid.) {
+entry:
+ %.global_tid..addr = alloca i32*, align 8
+ %.bound_tid..addr = alloca i32*, align 8
+ store i32* %.global_tid., i32** %.global_tid..addr, align 8
+ store i32* %.bound_tid., i32** %.bound_tid..addr, align 8
+ ret void
+}
+
+declare !callback !2 void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
+
+define dso_local void @live_fork_call() {
+entry:
+ call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @0, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @.omp_outlined..1 to void (i32*, i32*, ...)*))
+ ret void
+}
+
+define internal void @.omp_outlined..1(i32* noalias %.global_tid., i32* noalias %.bound_tid.) {
+entry:
+ %.global_tid..addr = alloca i32*, align 8
+ %.bound_tid..addr = alloca i32*, align 8
+ store i32* %.global_tid., i32** %.global_tid..addr, align 8
+ store i32* %.bound_tid., i32** %.bound_tid..addr, align 8
+ call void (...) @d()
+ ret void
+}
+
+declare dso_local void @d(...)
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 11.0.0"}
+!2 = !{!3}
+!3 = !{i64 2, i64 -1, i64 -1, i1 true}
More information about the llvm-commits
mailing list