Skip to content

Commit

Permalink
[CallGraph] Ignore callback uses
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ggeorgakoudis committed Jul 14, 2020
1 parent b98f414 commit aef60af
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 7 deletions.
6 changes: 4 additions & 2 deletions llvm/include/llvm/IR/Function.h
Expand Up @@ -830,9 +830,11 @@ class Function : public GlobalObject, public ilist_node<Function> {

/// 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
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Analysis/CallGraph.cpp
Expand Up @@ -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);
Expand Down
14 changes: 12 additions & 2 deletions llvm/lib/IR/Function.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -1484,12 +1485,21 @@ Optional<Function *> 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<BlockAddress>(FU))
continue;

if (IgnoreCallbackUses) {
AbstractCallSite ACS(&U);
if (ACS && ACS.isCallbackCall())
continue;
}

const auto *Call = dyn_cast<CallBase>(FU);
if (!Call) {
if (PutOffender)
Expand Down
51 changes: 51 additions & 0 deletions llvm/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 <<null function>><<{{.*}}>> #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}

0 comments on commit aef60af

Please sign in to comment.