From f60a7e942d7fae43bcf4b799e5ec67928856bd83 Mon Sep 17 00:00:00 2001 From: Giorgis Georgakoudis Date: Tue, 7 Jul 2020 22:43:24 -0700 Subject: [PATCH] [CallGraph] Ignore callback uses Summary: Ignore callback uses when adding a callback function in the CallGraph. Callback functions are typically created when outlining, e.g. for OpenMP, so they have internal scope and linkage. They should not be added to the ExternalCallingNode since they are only callable by the specified caller function at creation time. A CGSCC pass, such as OpenMPOpt, may need to update the CallGraph by adding a new outlined callback function. Without ignoring callback uses, adding breaks CGSCC pass restrictions and results to a broken CallGraph. Reviewers: jdoerfert Subscribers: hiraditya, sstefan1, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D83370 --- include/llvm/IR/Function.h | 6 ++- lib/Analysis/CallGraph.cpp | 7 +-- lib/IR/Function.cpp | 14 ++++- .../CallGraph/ignore-callback-uses.ll | 51 +++++++++++++++++++ 4 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 test/Analysis/CallGraph/ignore-callback-uses.ll diff --git a/include/llvm/IR/Function.h b/include/llvm/IR/Function.h index ee66abc3eae..bb4ec13c761 100644 --- a/include/llvm/IR/Function.h +++ b/include/llvm/IR/Function.h @@ -830,9 +830,11 @@ public: /// hasAddressTaken - returns true if there are any uses of this function /// other than direct calls or invokes to it, or blockaddress expressions. - /// Optionally passes back an offending user for diagnostic purposes. + /// Optionally passes back an offending user for diagnostic purposes and + /// ignores callback uses. /// - bool hasAddressTaken(const User** = nullptr) const; + bool hasAddressTaken(const User ** = nullptr, + bool IgnoreCallbackUses = false) const; /// isDefTriviallyDead - Return true if it is trivially safe to remove /// this function definition from the module (because it isn't externally diff --git a/lib/Analysis/CallGraph.cpp b/lib/Analysis/CallGraph.cpp index d8abccfdb09..55adb454b73 100644 --- a/lib/Analysis/CallGraph.cpp +++ b/lib/Analysis/CallGraph.cpp @@ -77,9 +77,10 @@ bool CallGraph::invalidate(Module &, const PreservedAnalyses &PA, void CallGraph::addToCallGraph(Function *F) { CallGraphNode *Node = getOrInsertFunction(F); - // If this function has external linkage or has its address taken, anything - // could call it. - if (!F->hasLocalLinkage() || F->hasAddressTaken()) + // If this function has external linkage or has its address taken and + // it is not a callback, then anything could call it. + if (!F->hasLocalLinkage() || + F->hasAddressTaken(nullptr, /*IgnoreCallbackUses=*/true)) ExternalCallingNode->addCalledFunction(nullptr, Node); populateCallGraphNode(Node); diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp index 0ec0cce83a8..10d535e3ab1 100644 --- a/lib/IR/Function.cpp +++ b/lib/IR/Function.cpp @@ -20,6 +20,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/IR/AbstractCallSite.h" #include "llvm/IR/Argument.h" #include "llvm/IR/Attributes.h" #include "llvm/IR/BasicBlock.h" @@ -1484,12 +1485,21 @@ Optional Intrinsic::remangleIntrinsicFunction(Function *F) { } /// hasAddressTaken - returns true if there are any uses of this function -/// other than direct calls or invokes to it. -bool Function::hasAddressTaken(const User* *PutOffender) const { +/// other than direct calls or invokes to it. Optionally ignores callback +/// uses. +bool Function::hasAddressTaken(const User **PutOffender, + bool IgnoreCallbackUses) const { for (const Use &U : uses()) { const User *FU = U.getUser(); if (isa(FU)) continue; + + if (IgnoreCallbackUses) { + AbstractCallSite ACS(&U); + if (ACS && ACS.isCallbackCall()) + continue; + } + const auto *Call = dyn_cast(FU); if (!Call) { if (PutOffender) diff --git a/test/Analysis/CallGraph/ignore-callback-uses.ll b/test/Analysis/CallGraph/ignore-callback-uses.ll new file mode 100644 index 00000000000..8964ca1efd8 --- /dev/null +++ b/test/Analysis/CallGraph/ignore-callback-uses.ll @@ -0,0 +1,51 @@ +; RUN: opt < %s -print-callgraph -disable-output 2>&1 | FileCheck %s +; CHECK: Call graph node <><<{{.*}}>> #uses=0 +; CHECK-NEXT: CS<{{.*}}> calls function 'f' +; CHECK-NEXT: CS<{{.*}}> calls function '__kmpc_fork_call' +; CHECK-EMPTY: + +%struct.ident_t = type { i32, i32, i32, i32, i8* } + +@0 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1 +@1 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @0, i32 0, i32 0) }, align 8 + +; Function Attrs: noinline nounwind optnone uwtable +define dso_local void @f() { +entry: + br label %omp_parallel + +omp_parallel: ; preds = %entry + call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @f..omp_par to void (i32*, i32*, ...)*)) + br label %omp.par.exit.split + +omp.par.exit.split: ; preds = %omp_parallel + ret void +} + +; Function Attrs: norecurse nounwind +define internal void @f..omp_par(i32* noalias %tid.addr, i32* noalias %zero.addr) { +omp.par.entry: + %tid.addr.local = alloca i32, align 4 + %0 = load i32, i32* %tid.addr, align 4 + store i32 %0, i32* %tid.addr.local, align 4 + %tid = load i32, i32* %tid.addr.local, align 4 + br label %omp.par.region + +omp.par.exit.split.exitStub: ; preds = %omp.par.outlined.exit + ret void + +omp.par.region: ; preds = %omp.par.entry + br label %omp.par.pre_finalize + +omp.par.pre_finalize: ; preds = %omp.par.region + br label %omp.par.outlined.exit + +omp.par.outlined.exit: ; preds = %omp.par.pre_finalize + br label %omp.par.exit.split.exitStub +} + +; Function Attrs: nounwind +declare !callback !2 void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) #2 + +!2 = !{!3} +!3 = !{i64 2, i64 -1, i64 -1, i1 true}