[ctx_prof] Automatically convert available external linkage to local for modules with contextual roots

[mtrofin]

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.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/109203.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/ElimAvailExtern.cpp (+8-5)
• (modified) llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll (+11-2)

diff --git a/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp b/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
index 2b34d3b5a56ea4..644effab9414ba 100644
--- a/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
+++ b/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
@@ -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"
@@ -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.
@@ -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);
@@ -125,8 +126,10 @@ 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);
+  if (!eliminateAvailableExternally(M, (CtxProf && !!(*CtxProf))))
+    ;
+  return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
diff --git a/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll b/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
index 786cc260d331c6..d0b96daf3bf3b1 100644
--- a/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
+++ b/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
@@ -1,6 +1,11 @@
 ; 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
+; 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, nothing gets converted.
+; 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)
 
@@ -12,13 +17,15 @@ 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]*}}()
@@ -26,4 +33,6 @@ define void @hello(ptr %g) {
 ; 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
\ No newline at end of file
+; CHECK: 1 elim-avail-extern - Number of functions removed
+
+; NOOP: 2 elim-avail-extern - Number of functions removed
\ No newline at end of file

[mtrofin]

• [ctx_prof] Automatically convert available external linkage to local for modules with contextual roots #109203 👈
• [ctx_prof] Handle select and its step instrumentation #109185
• [nfc][ctx_prof] Don't try finding callsite annotation for un-instrumentable callsites #109184
• [ctx_prof] Fix ProfileAnnotator::allTakenPathsExit #109183
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mtrofin and the rest of your teammates on Graphite

[teresajohnson]

lgtm with a couple of comment suggestions

[mtrofin]

Merge activity

• Sep 24, 4:17 PM EDT: @mtrofin started a stack merge that includes this pull request via Graphite.
• Sep 24, 4:18 PM EDT: @mtrofin merged this pull request with Graphite.

[mikaelholmen]

If I compile with EXPENSIVE_CHECKS on a large number of lit tests fail with this patch.
E.g.
build-all-expensive/bin/llvm-lit -av ../lld/test/wasm/lto/thin-archivecollision.ll
fails with

LLVM ERROR: Function @blah changed by EliminateAvailableExternallyPass without invalidating analyses

I suppose this is due to the error @RKSimon pointed out in
#109203 (review)