Skip to content

Commit

Permalink
[ctx_prof] Automatically convert available external linkage to local …
Browse files Browse the repository at this point in the history
…for modules with contextual roots (llvm#109203)

For the modules containing context roots, the way IPO happens will potentially result in imported functions that are differently specialized (even if themselves not inlined) than their originals. So we want to convert them to local rather than elide them.

Eventually we'd perform this as a ThinLTO directive.
  • Loading branch information
mtrofin authored Sep 24, 2024
1 parent 38371a1 commit 4911235
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
19 changes: 14 additions & 5 deletions llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "llvm/Transforms/IPO/ElimAvailExtern.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/CtxProfAnalysis.h"
#include "llvm/IR/Constant.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Function.h"
Expand Down Expand Up @@ -88,7 +89,7 @@ static void convertToLocalCopy(Module &M, Function &F) {
++NumConversions;
}

static bool eliminateAvailableExternally(Module &M) {
static bool eliminateAvailableExternally(Module &M, bool Convert) {
bool Changed = false;

// Drop initializers of available externally global variables.
Expand All @@ -112,7 +113,7 @@ static bool eliminateAvailableExternally(Module &M) {
if (F.isDeclaration() || !F.hasAvailableExternallyLinkage())
continue;

if (ConvertToLocal)
if (Convert || ConvertToLocal)
convertToLocalCopy(M, F);
else
deleteFunction(F);
Expand All @@ -125,8 +126,16 @@ static bool eliminateAvailableExternally(Module &M) {
}

PreservedAnalyses
EliminateAvailableExternallyPass::run(Module &M, ModuleAnalysisManager &) {
if (!eliminateAvailableExternally(M))
return PreservedAnalyses::all();
EliminateAvailableExternallyPass::run(Module &M, ModuleAnalysisManager &MAM) {
auto *CtxProf = MAM.getCachedResult<CtxProfAnalysis>(M);
// Convert to local instead of eliding if we use contextual profiling in this
// module. This is because the IPO decisions performed with contextual
// information will likely differ from decisions made without. For a function
// that's imported, its optimizations will, thus, differ, and be specialized
// for this contextual information. Eliding it in favor of the original would
// undo these optimizations.
if (!eliminateAvailableExternally(M, /*Convert=*/(CtxProf && !!(*CtxProf))))
;
return PreservedAnalyses::all();
return PreservedAnalyses::none();
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
; REQUIRES: asserts
; RUN: opt -passes=elim-avail-extern -avail-extern-to-local -stats -S 2>&1 < %s | FileCheck %s
;
; RUN: echo '[{"Guid":1234, "Counters": [1]}]' | llvm-ctxprof-util fromJSON --input=- --output=%t_profile.ctxprofdata
;
; Because we pass a contextual profile with a root defined in this module, we expect the outcome to be the same as-if
; we passed -avail-extern-to-local, i.e. available_externally don't get elided and instead get converted to local linkage
; RUN: opt -passes='assign-guid,require<ctx-prof-analysis>,elim-avail-extern' -use-ctx-profile=%t_profile.ctxprofdata -stats -S 2>&1 < %s | FileCheck %s

; If the profile doesn't apply to this module, available_externally won't get converted to internal linkage, and will be
; removed instead.
; RUN: echo '[{"Guid":5678, "Counters": [1]}]' | llvm-ctxprof-util fromJSON --input=- --output=%t_profile_bad.ctxprofdata
; RUN: opt -passes='assign-guid,require<ctx-prof-analysis>,elim-avail-extern' -use-ctx-profile=%t_profile_bad.ctxprofdata -stats -S 2>&1 < %s | FileCheck %s --check-prefix=NOOP

declare void @call_out(ptr %fct)

Expand All @@ -12,18 +22,22 @@ define available_externally hidden void @g() {
ret void
}

define void @hello(ptr %g) {
define void @hello(ptr %g) !guid !0 {
call void @f()
%f = load ptr, ptr @f
call void @call_out(ptr %f)
ret void
}

!0 = !{i64 1234}

; CHECK: define internal void @f.__uniq.{{[0-9|a-f]*}}()
; CHECK: declare hidden void @g()
; CHECK: call void @f.__uniq.{{[0-9|a-f]*}}()
; CHECK-NEXT: load ptr, ptr @f
; CHECK-NEXT: call void @call_out(ptr %f)
; CHECK: Statistics Collected
; CHECK: 1 elim-avail-extern - Number of functions converted
; CHECK: 1 elim-avail-extern - Number of functions removed
; CHECK: 1 elim-avail-extern - Number of functions removed

; NOOP: 2 elim-avail-extern - Number of functions removed

0 comments on commit 4911235

Please sign in to comment.