Skip to content

[EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines #92171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
May 31, 2024
Merged

[EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines #92171

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
df3f8df
wip: Move instrumentation passes
pasko May 6, 2024
3ea4de1
Address the first round of review comments
pasko May 15, 2024
c3d3637
Address 2nd round of comments
pasko May 16, 2024
5d46226
Run prelink instrumentation without LTO, plus tests
pasko May 21, 2024
5cabc69
Address review comments for the new tests
pasko May 22, 2024
e9c64d5
Remove a couple of clang tests
pasko May 22, 2024
d095f41
Add one more -O0 check in pre-inliner
pasko May 22, 2024
e5da299
Revert "Remove a couple of clang tests"
pasko May 23, 2024
1a4e5fb
Address comments in new tests
pasko May 23, 2024
ee48568
Merge branch 'main' into the instrumentation WIP PR
pasko May 23, 2024
fd92b90
For recovered tests in Clang only check attributes and disable llvm p…
pasko May 23, 2024
b65cec9
Convert more Clang tests to only check attributes
pasko May 24, 2024
9996036
Merge branch 'main'
pasko May 27, 2024
1abdb86
Run clang/test/CodeGen/... instrumentation tests only with -disable-l…
pasko May 27, 2024
27359fe
Move EntryExitInstrumenterPass to EarlyFPM
pasko May 27, 2024
bee1fb2
remove unnecessary condition
pasko May 29, 2024
e3f2097
Adjust passes in tests
pasko May 30, 2024
3579930
Whitespace beautify
pasko May 30, 2024
9270233
Add the pass in llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
pasko May 31, 2024
34658c6
Merge 'main'
pasko May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions clang/lib/CodeGen/BackendUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
#include "llvm/Transforms/Scalar/GVN.h"
#include "llvm/Transforms/Scalar/JumpThreading.h"
#include "llvm/Transforms/Utils/Debugify.h"
#include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
#include <memory>
#include <optional>
Expand Down Expand Up @@ -987,22 +986,6 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
/*DropTypeTests=*/true));
});

if (CodeGenOpts.InstrumentFunctions ||
CodeGenOpts.InstrumentFunctionEntryBare ||
CodeGenOpts.InstrumentFunctionsAfterInlining ||
CodeGenOpts.InstrumentForProfiling) {
PB.registerPipelineStartEPCallback(
[](ModulePassManager &MPM, OptimizationLevel Level) {
MPM.addPass(createModuleToFunctionPassAdaptor(
EntryExitInstrumenterPass(/*PostInlining=*/false)));
});
PB.registerOptimizerLastEPCallback(
[](ModulePassManager &MPM, OptimizationLevel Level) {
MPM.addPass(createModuleToFunctionPassAdaptor(
EntryExitInstrumenterPass(/*PostInlining=*/true)));
});
}

// Register callbacks to schedule sanitizer passes at the appropriate part
// of the pipeline.
if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
Expand Down
26 changes: 8 additions & 18 deletions clang/test/CodeGen/X86/x86_64-instrument-functions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,27 @@
// REQUIRES: x86-registered-target
// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s
// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s
// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s
// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s
// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s
// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s

__attribute__((always_inline)) int leaf(int x) {
return x;
// CHECK-LABEL: define {{.*}} @leaf
// CHECK: call void @__cyg_profile_func_enter
// CHECK-NOT: cyg_profile
// CHECK: call void @__cyg_profile_func_exit
// CHECK-NOT: cyg_profile
// CHECK: ret
}

int root(int x) {
return leaf(x);
// CHECK-LABEL: define {{.*}} @root
// CHECK: call void @__cyg_profile_func_enter
// CHECK-NOT: cyg_profile

// Inlined from leaf():
// CHECK: call void @__cyg_profile_func_enter
// CHECK-NOT: cyg_profile
// CHECK: call void @__cyg_profile_func_exit
// CHECK-NOT: cyg_profile

// CHECK: call void @__cyg_profile_func_exit
// CHECK: ret

// NOINLINE-LABEL: define {{.*}} @root
// NOINLINE: call void @__cyg_profile_func_enter
// NOINLINE-NOT: cyg_profile
// NOINLINE: call void @__cyg_profile_func_exit
// NOINLINE-NOT: cyg_profile
// NOINLINE: ret
}

// CHECK: attributes #0 = { alwaysinline {{.*}} "instrument-function-entry"="__cyg_profile_func_enter" "instrument-function-exit"="__cyg_profile_func_exit"
// CHECK: attributes #1 = { {{.*}} "instrument-function-entry"="__cyg_profile_func_enter" "instrument-function-exit"="__cyg_profile_func_exit"
// NOINLINE: attributes #0 = { alwaysinline {{.*}} "instrument-function-entry-inlined"="__cyg_profile_func_enter" "instrument-function-exit-inlined"="__cyg_profile_func_exit"
// NOINLINE: attributes #1 = { {{.*}} "instrument-function-entry-inlined"="__cyg_profile_func_enter" "instrument-function-exit-inlined"="__cyg_profile_func_exit"
64 changes: 33 additions & 31 deletions clang/test/Frontend/gnu-mcount.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,32 @@
// REQUIRES: arm-registered-target,aarch64-registered-target

// RUN: %clang -target armv7-unknown-none-eabi -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -target armv7-unknown-none-eabi -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang --target=aarch64-unknown-none-gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT
// RUN: %clang -target armv7-unknown-linux-gnueabi -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI
// RUN: %clang -target armv7-unknown-linux-gnueabi -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-MEABI-GNU
// RUN: %clang --target=aarch64-unknown-linux -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER
// RUN: %clang -target armv7-unknown-linux-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI
// RUN: %clang -target armv7-unknown-linux-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-MEABI-GNU
// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang --target=aarch64-unknown-freebsd -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-EABI-FREEBSD
// RUN: %clang -target armv7-unknown-openbsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER
// RUN: %clang -target armv7-unknown-openbsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER
// RUN: %clang --target=aarch64-unknown-openbsd -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER
// RUN: %clang -target armv7-unknown-netbsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -target armv7-unknown-netbsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang --target=aarch64-unknown-netbsd -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -target armv7-apple-ios -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -target armv7-apple-ios -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -target arm64-apple-ios -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -target arm64-apple-ios -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -target armv7-unknown-rtems-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT
// RUN: %clang -target armv7-unknown-rtems-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT
// RUN: %clang --target=aarch64-unknown-rtems -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,MCOUNT
// RUN: %clang --target=aarch64-unknown-rtems -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,MCOUNT
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-none-eabi -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-none-eabi -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-none-gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-linux-gnueabi -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-linux-gnueabi -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-MEABI-GNU
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-linux -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-linux-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-linux-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-MEABI-GNU
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-freebsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-freebsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-freebsd -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-EABI-FREEBSD
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-openbsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-openbsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-openbsd -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-netbsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-netbsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-netbsd -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-apple-ios -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-apple-ios -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -Xclang -disable-llvm-passes -target arm64-apple-ios -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -Xclang -disable-llvm-passes -target arm64-apple-ios -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-rtems-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT
// RUN: %clang -Xclang -disable-llvm-passes -target armv7-unknown-rtems-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-rtems -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,MCOUNT
// RUN: %clang -Xclang -disable-llvm-passes --target=aarch64-unknown-rtems -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,MCOUNT

// Correct function name should be used for mcount instrumentation call.

int f() {
return 0;
Expand All @@ -33,9 +35,9 @@ int f() {
// CHECK-LABEL: f
// TODO: add profiling support for arm-baremetal
// UNSUPPORTED-NOT: call void
// CHECK-ARM-EABI: call void @"\01mcount"()
// MCOUNT: call void @mcount()
// UNDER: call void @"\01_mcount"()
// UNDER_UNDER: call void @__mcount()
// CHECK-ARM64-EABI-FREEBSD: call void @.mcount()
// CHECK-ARM-EABI-MEABI-GNU: call void @llvm.arm.gnu.eabi.mcount()
// CHECK-ARM-EABI: attributes #0 = {{.*}} "instrument-function-entry-inlined"="\01mcount"
// MCOUNT: attributes #0 = {{.*}} "instrument-function-entry-inlined"="mcount"
// UNDER: attributes #0 = {{.*}} "instrument-function-entry-inlined"="\01_mcount"
// UNDER_UNDER: attributes #0 = {{.*}} "instrument-function-entry-inlined"="__mcount"
// CHECK-ARM64-EABI-FREEBSD: attributes #0 = {{.*}} "instrument-function-entry-inlined"=".mcount"
// CHECK-ARM-EABI-MEABI-GNU: attributes #0 = {{.*}} "instrument-function-entry-inlined"="llvm.arm.gnu.eabi.mcount"
1 change: 1 addition & 0 deletions llvm/include/llvm/InitializePasses.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ void initializePostDomOnlyViewerWrapperPassPass(PassRegistry &);
void initializePostDomPrinterWrapperPassPass(PassRegistry &);
void initializePostDomViewerWrapperPassPass(PassRegistry &);
void initializePostDominatorTreeWrapperPassPass(PassRegistry&);
void initializePostInlineEntryExitInstrumenterPass(PassRegistry&);
void initializePostMachineSchedulerPass(PassRegistry&);
void initializePostRAHazardRecognizerPass(PassRegistry&);
void initializePostRAMachineSinkingPass(PassRegistry&);
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/LinkAllPasses.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ namespace {
(void)llvm::createTLSVariableHoistPass();
(void) llvm::createConstantHoistingPass();
(void)llvm::createCodeGenPrepareLegacyPass();
(void) llvm::createPostInlineEntryExitInstrumenterPass();
(void) llvm::createEarlyCSEPass();
(void) llvm::createGVNPass();
(void) llvm::createPostDomTree();
Expand Down
9 changes: 9 additions & 0 deletions llvm/include/llvm/Transforms/Utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ extern char &LowerInvokePassID;
FunctionPass *createLowerSwitchPass();
extern char &LowerSwitchID;

//===----------------------------------------------------------------------===//
//
// EntryExitInstrumenter pass - Instrument function entry/exit with calls to
// mcount(), @__cyg_profile_func_{enter,exit} and the like. There are two
// variants, intended to run pre- and post-inlining, respectively. Only the
// post-inlining variant is used with the legacy pass manager.
//
FunctionPass *createPostInlineEntryExitInstrumenterPass();

//===----------------------------------------------------------------------===//
//
// BreakCriticalEdges - Break all of the critical edges in the CFG by inserting
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/CodeGen/TargetPassConfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,9 @@ void TargetPassConfig::addIRPasses() {
// passes since it emits those kinds of intrinsics.
addPass(createExpandVectorPredicationPass());

// Instrument function entry after all inlining.
addPass(createPostInlineEntryExitInstrumenterPass());

// Add scalarization of target's unsupported masked memory intrinsics pass.
// the unsupported intrinsic will be replaced with a chain of basic blocks,
// that stores/loads element one-by-one if the appropriate mask bit is set.
Expand Down
16 changes: 16 additions & 0 deletions llvm/lib/Passes/PassBuilderPipelines.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
#include "llvm/Transforms/Utils/AssumeBundleBuilder.h"
#include "llvm/Transforms/Utils/CanonicalizeAliases.h"
#include "llvm/Transforms/Utils/CountVisits.h"
#include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
#include "llvm/Transforms/Utils/InjectTLIMappings.h"
#include "llvm/Transforms/Utils/LibCallsShrinkWrap.h"
#include "llvm/Transforms/Utils/Mem2Reg.h"
Expand Down Expand Up @@ -395,6 +396,11 @@ static bool isLTOPreLink(ThinOrFullLTOPhase Phase) {
Phase == ThinOrFullLTOPhase::FullLTOPreLink;
}

static bool isLTOPostLink(ThinOrFullLTOPhase Phase) {
return Phase == ThinOrFullLTOPhase::ThinLTOPostLink ||
Phase == ThinOrFullLTOPhase::FullLTOPostLink;
}

// TODO: Investigate the cost/benefit of tail call elimination on debugging.
FunctionPassManager
PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
Expand Down Expand Up @@ -1030,6 +1036,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
Phase != ThinOrFullLTOPhase::ThinLTOPostLink)
MPM.addPass(SampleProfileProbePass(TM));

// Instrument function entry and exit before all inlining.
if (!isLTOPostLink(Phase)) {
MPM.addPass(createModuleToFunctionPassAdaptor(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually is it possible to add this into EarlyFPM below? it's nice to have fewer module->function adaptors. (e.g. in EarlyFPM we run all the function passes on a single function before moving to the next one which is nice for memory locality)

sorry for suggesting this so late, since it'll require updating all the pipeline tests... but first check that none of the other tests fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this, and all non-pipeline tests pass. Though in my local FDO+ThinLTO reproducer I saw this causes __cyg_profile_func_enter_bare to be inserted multiple times per toplevel function.

This behaviour surprised me. The bare instrumentation does not move with the change. Is removing the module to function pass adaptor making something easier to inline later on?

I'd like to investigate, but I am afraid I'll need some clues :)

The change:

diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 3a7634f90c07..cae340a19fb6 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -397,11 +397,6 @@ static bool isLTOPreLink(ThinOrFullLTOPhase Phase) {
          Phase == ThinOrFullLTOPhase::FullLTOPreLink;
 }
 
-static bool isLTOPostLink(ThinOrFullLTOPhase Phase) {
-  return Phase == ThinOrFullLTOPhase::ThinLTOPostLink ||
-         Phase == ThinOrFullLTOPhase::FullLTOPostLink;
-}
-
 // TODO: Investigate the cost/benefit of tail call elimination on debugging.
 FunctionPassManager
 PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
@@ -1039,12 +1034,6 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
       Phase != ThinOrFullLTOPhase::ThinLTOPostLink)
     MPM.addPass(SampleProfileProbePass(TM));
 
-  // Instrument function entry and exit before all inlining.
-  if (!isLTOPostLink(Phase)) {
-    MPM.addPass(createModuleToFunctionPassAdaptor(
-        EntryExitInstrumenterPass(/*PostInlining=*/false)));
-  }
-
   bool HasSampleProfile = PGOOpt && (PGOOpt->Action == PGOOptions::SampleUse);
 
   // In ThinLTO mode, when flattened profile is used, all the available
@@ -1081,6 +1070,9 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
     MPM.addPass(CoroEarlyPass());
 
     FunctionPassManager EarlyFPM;
+    if (Phase != ThinOrFullLTOPhase::FullLTOPostLink) {
+      EarlyFPM.addPass(EntryExitInstrumenterPass(/*PostInlining=*/false));
+    }
     // Lower llvm.expect to metadata before attempting transforms.
     // Compare/branch metadata may alter the behavior of passes like
     // SimplifyCFG.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the check for Phase != ThinOrFullLTOPhase::FullLTOPostLink is unnecessary, buildModuleSimplificationPipeline isn't called for FullLTO post link

I'm not sure why you're seeing that behavior if we're only running the post-inline instrumenter once in the codegen pipeline

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you have a small local repro, you can pass -Wl,-mllvm,-print-after-all to the link command to have it print IR after every pass and see where the calls are getting added. or to see changes in the pre-link optimization pipeline, -mllvm=-print-after-all (or -mllvm=-print-changed=quiet, which doesn't work with the IR parts of the codegen pipeline for reasons)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the check for Phase != ThinOrFullLTOPhase::FullLTOPostLink is unnecessary, buildModuleSimplificationPipeline isn't called for FullLTO post link

Removed. Indeed there is an assertion for this a few lines prior.

I'm not sure why you're seeing that behavior if we're only running the post-inline instrumenter once in the codegen pipeline

Apparently I made a mistake when building the reproducer. When checking today I saw everything working as expected, i.e. <=1 bare hooks inserted per function in the final DSO. Sorry for the noise.

EntryExitInstrumenterPass(/*PostInlining=*/false)));
}

bool HasSampleProfile = PGOOpt && (PGOOpt->Action == PGOOptions::SampleUse);

// In ThinLTO mode, when flattened profile is used, all the available
Expand Down Expand Up @@ -2048,6 +2060,10 @@ ModulePassManager PassBuilder::buildO0DefaultPipeline(OptimizationLevel Level,
/*IsCS=*/false, PGOOpt->AtomicCounterUpdate, PGOOpt->ProfileFile,
PGOOpt->ProfileRemappingFile, PGOOpt->FS);

// Instrument function entry and exit before all inlining.
MPM.addPass(createModuleToFunctionPassAdaptor(
EntryExitInstrumenterPass(/*PostInlining=*/false)));

invokePipelineStartEPCallbacks(MPM, Level);

if (PGOOpt && PGOOpt->DebugInfoForProfiling)
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Scalar/Scalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,5 @@ void llvm::initializeScalarOpts(PassRegistry &Registry) {
initializeSpeculativeExecutionLegacyPassPass(Registry);
initializeStraightLineStrengthReduceLegacyPassPass(Registry);
initializePlaceBackedgeSafepointsLegacyPassPass(Registry);
initializePostInlineEntryExitInstrumenterPass(Registry);
}
35 changes: 35 additions & 0 deletions llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Type.h"
#include "llvm/InitializePasses.h"
#include "llvm/TargetParser/Triple.h"
#include "llvm/Pass.h"
#include "llvm/Transforms/Utils.h"

using namespace llvm;

Expand Down Expand Up @@ -135,6 +138,38 @@ static bool runOnFunction(Function &F, bool PostInlining) {
return Changed;
}

namespace {
struct PostInlineEntryExitInstrumenter : public FunctionPass {
static char ID;
PostInlineEntryExitInstrumenter() : FunctionPass(ID) {
initializePostInlineEntryExitInstrumenterPass(
*PassRegistry::getPassRegistry());
}
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addPreserved<GlobalsAAWrapperPass>();
AU.addPreserved<DominatorTreeWrapperPass>();
}
bool runOnFunction(Function &F) override { return ::runOnFunction(F, true); }
};
char PostInlineEntryExitInstrumenter::ID = 0;
}

INITIALIZE_PASS_BEGIN(
PostInlineEntryExitInstrumenter, "post-inline-ee-instrument",
"Instrument function entry/exit with calls to e.g. mcount() "
"(post inlining)",
false, false)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
INITIALIZE_PASS_END(
PostInlineEntryExitInstrumenter, "post-inline-ee-instrument",
"Instrument function entry/exit with calls to e.g. mcount() "
"(post inlining)",
false, false)

FunctionPass *llvm::createPostInlineEntryExitInstrumenterPass() {
return new PostInlineEntryExitInstrumenter();
}

PreservedAnalyses
llvm::EntryExitInstrumenterPass::run(Function &F, FunctionAnalysisManager &AM) {
if (!runOnFunction(F, PostInlining))
Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/AArch64/O0-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
; CHECK-NEXT: Lower constant intrinsics
; CHECK-NEXT: Remove unreachable blocks from the CFG
; CHECK-NEXT: Expand vector predication intrinsics
; CHECK-NEXT: Instrument function entry/exit with calls to e.g. mcount() (post inlining)
; CHECK-NEXT: Scalarize Masked Memory Intrinsics
; CHECK-NEXT: Expand reduction intrinsics
; CHECK-NEXT: AArch64 Globals Tagging
Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/AArch64/O3-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
; CHECK-NEXT: Replace intrinsics with calls to vector library
; CHECK-NEXT: Partially inline calls to library functions
; CHECK-NEXT: Expand vector predication intrinsics
; CHECK-NEXT: Instrument function entry/exit with calls to e.g. mcount() (post inlining)
; CHECK-NEXT: Scalarize Masked Memory Intrinsics
; CHECK-NEXT: Expand reduction intrinsics
; CHECK-NEXT: Natural Loop Information
Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/ARM/O3-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
; CHECK-NEXT: Replace intrinsics with calls to vector library
; CHECK-NEXT: Partially inline calls to library functions
; CHECK-NEXT: Expand vector predication intrinsics
; CHECK-NEXT: Instrument function entry/exit with calls to e.g. mcount() (post inlining)
; CHECK-NEXT: Scalarize Masked Memory Intrinsics
; CHECK-NEXT: Expand reduction intrinsics
; CHECK-NEXT: Natural Loop Information
Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/LoongArch/O0-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
; CHECK-NEXT: Lower constant intrinsics
; CHECK-NEXT: Remove unreachable blocks from the CFG
; CHECK-NEXT: Expand vector predication intrinsics
; CHECK-NEXT: Instrument function entry/exit with calls to e.g. mcount() (post inlining)
; CHECK-NEXT: Scalarize Masked Memory Intrinsics
; CHECK-NEXT: Expand reduction intrinsics
; CHECK-NEXT: Exception handling preparation
Expand Down
Loading