[llvm-branch-commits] [llvm] [PGO] Generate __llvm_profile_raw_version only when instrumented (PR #93917)
Pavel Samolysov via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu May 30 20:51:04 PDT 2024
https://github.com/samolisov created https://github.com/llvm/llvm-project/pull/93917
Currently, only modules that contain at least a single function
definition are instrumented. When a module contains no function
definitions at all, the module is not instrumented (yet?) and there is
no reason to introduce the '__llvm_profile_raw_version' variable into
the module.
>From ac467402f0d688c8295bbca0f03161516b6982df Mon Sep 17 00:00:00 2001
From: Pavel Samolysov <samolisov at gmail.com>
Date: Fri, 31 May 2024 05:46:25 +0300
Subject: [PATCH 1/2] [PGO] Preserve analysis results when nothing was
instrumented
The 'PGOInstrumentationGen' pass should preserve all analysis results
when nothing was actually instrumented. Currently, only modules that
contain at least a single function definition are instrumented. When a
module contains only function declarations and, optionally, global
variable definitions (a module for the regular-LTO phase for thin-LTO
when LTOUnit splitting is enabled, for example), such module is not
instrumented (yet?) and there is no reason to invalidate any analysis
results.
NFC.
---
.../Instrumentation/PGOInstrumentation.cpp | 5 ++
.../PGOInstrumentationTest.cpp | 66 ++++++++++++++-----
2 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 2269c2e0fffae..fbf9688ed2d1e 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1845,6 +1845,7 @@ static bool InstrumentAllFunctions(
std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
collectComdatMembers(M, ComdatMembers);
+ bool AnythingInstrumented = false;
for (auto &F : M) {
if (skipPGOGen(F))
continue;
@@ -1852,7 +1853,11 @@ static bool InstrumentAllFunctions(
auto *BPI = LookupBPI(F);
auto *BFI = LookupBFI(F);
instrumentOneFunc(F, &M, TLI, BPI, BFI, ComdatMembers, IsCS);
+ AnythingInstrumented = true;
}
+ if (!AnythingInstrumented)
+ return false;
+
return true;
}
diff --git a/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp b/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp
index 02c2df2a138b0..cbeaa501d4d88 100644
--- a/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp
+++ b/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp
@@ -104,9 +104,13 @@ class MockModuleAnalysisHandle
ModuleAnalysisManager::Invalidator &));
};
-struct PGOInstrumentationGenTest
- : public Test,
- WithParamInterface<std::tuple<StringRef, StringRef>> {
+template <typename ParamType> struct PGOTestName {
+ std::string operator()(const TestParamInfo<ParamType> &Info) const {
+ return std::get<1>(Info.param).str();
+ }
+};
+
+struct PGOInstrumentationGenTest : public Test {
LLVMContext Ctx;
ModulePassManager MPM;
PassBuilder PB;
@@ -143,12 +147,47 @@ struct PGOInstrumentationGenTest
}
};
+struct PGOInstrumentationGenInstrumentTest
+ : PGOInstrumentationGenTest,
+ WithParamInterface<std::tuple<StringRef, StringRef>> {};
+
static constexpr StringRef CodeWithFuncDefs = R"(
define i32 @f(i32 %n) {
entry:
ret i32 0
})";
+INSTANTIATE_TEST_SUITE_P(
+ PGOInstrumetationGenTestSuite, PGOInstrumentationGenInstrumentTest,
+ Values(std::make_tuple(CodeWithFuncDefs, "instrument_function_defs")),
+ PGOTestName<PGOInstrumentationGenInstrumentTest::ParamType>());
+
+TEST_P(PGOInstrumentationGenInstrumentTest, Instrumented) {
+ const StringRef Code = std::get<0>(GetParam());
+ parseAssembly(Code);
+
+ ASSERT_THAT(M, NotNull());
+
+ Sequence PassSequence;
+ EXPECT_CALL(MMAHandle, run(Ref(*M), _))
+ .InSequence(PassSequence)
+ .WillOnce(DoDefault());
+ EXPECT_CALL(MMAHandle, invalidate(Ref(*M), _, _))
+ .InSequence(PassSequence)
+ .WillOnce(DoDefault());
+
+ MPM.run(*M, MAM);
+
+ const auto *IRInstrVar =
+ M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
+ EXPECT_THAT(IRInstrVar, NotNull());
+ EXPECT_FALSE(IRInstrVar->isDeclaration());
+}
+
+struct PGOInstrumentationGenIgnoreTest
+ : PGOInstrumentationGenTest,
+ WithParamInterface<std::tuple<StringRef, StringRef>> {};
+
static constexpr StringRef CodeWithFuncDecls = R"(
declare i32 @f(i32);
)";
@@ -159,27 +198,20 @@ static constexpr StringRef CodeWithGlobals = R"(
)";
INSTANTIATE_TEST_SUITE_P(
- PGOInstrumetationGenTestSuite, PGOInstrumentationGenTest,
- Values(std::make_tuple(CodeWithFuncDefs, "instrument_function_defs"),
- std::make_tuple(CodeWithFuncDecls, "instrument_function_decls"),
+ PGOInstrumetationGenIgnoreTestSuite, PGOInstrumentationGenIgnoreTest,
+ Values(std::make_tuple(CodeWithFuncDecls, "instrument_function_decls"),
std::make_tuple(CodeWithGlobals, "instrument_globals")),
- [](const TestParamInfo<PGOInstrumentationGenTest::ParamType> &Info) {
- return std::get<1>(Info.param).str();
- });
+ PGOTestName<PGOInstrumentationGenIgnoreTest::ParamType>());
-TEST_P(PGOInstrumentationGenTest, Instrumented) {
+TEST_P(PGOInstrumentationGenIgnoreTest, NotInstrumented) {
const StringRef Code = std::get<0>(GetParam());
+
parseAssembly(Code);
ASSERT_THAT(M, NotNull());
- Sequence PassSequence;
- EXPECT_CALL(MMAHandle, run(Ref(*M), _))
- .InSequence(PassSequence)
- .WillOnce(DoDefault());
- EXPECT_CALL(MMAHandle, invalidate(Ref(*M), _, _))
- .InSequence(PassSequence)
- .WillOnce(DoDefault());
+ EXPECT_CALL(MMAHandle, run(Ref(*M), _)).WillOnce(DoDefault());
+ EXPECT_CALL(MMAHandle, invalidate(Ref(*M), _, _)).Times(0);
MPM.run(*M, MAM);
>From 5d5ead1dbd9aac486aada3acf81cc09464ab7bae Mon Sep 17 00:00:00 2001
From: Pavel Samolysov <samolisov at gmail.com>
Date: Fri, 31 May 2024 05:52:00 +0300
Subject: [PATCH 2/2] [PGO] Generate __llvm_profile_raw_version only when
instrumented
Currently, only modules that contain at least a single function
definition are instrumented. When a module contains no function
definitions at all, the module is not instrumented (yet?) and there is
no reason to introduce the '__llvm_profile_raw_version' variable into
the module.
---
.../Instrumentation/PGOInstrumentation.cpp | 20 ++++++++++++-------
.../PGOProfile/declarations_only.ll | 7 +------
.../PGOProfile/global_variables_only.ll | 5 +----
.../PGOProfile/no_global_variables.ll | 15 ++++++++++++++
.../PGOInstrumentationTest.cpp | 3 +--
5 files changed, 31 insertions(+), 19 deletions(-)
create mode 100644 llvm/test/Transforms/PGOProfile/no_global_variables.ll
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index fbf9688ed2d1e..ff94fc98d0117 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -405,9 +405,14 @@ static GlobalVariable *createIRLevelProfileFlagVar(Module &M, bool IsCS) {
ProfileVersion |= VARIANT_MASK_BYTE_COVERAGE;
if (PGOTemporalInstrumentation)
ProfileVersion |= VARIANT_MASK_TEMPORAL_PROF;
- auto IRLevelVersionVariable = new GlobalVariable(
+ assert(!M.global_empty() &&
+ "If a module was instrumented, there must be defined global variables "
+ "at least for the counters.");
+ auto *InsertionPoint = &*M.global_begin();
+ auto *IRLevelVersionVariable = new GlobalVariable(
M, IntTy64, true, GlobalValue::WeakAnyLinkage,
- Constant::getIntegerValue(IntTy64, APInt(64, ProfileVersion)), VarName);
+ Constant::getIntegerValue(IntTy64, APInt(64, ProfileVersion)), VarName,
+ InsertionPoint);
IRLevelVersionVariable->setVisibility(GlobalValue::HiddenVisibility);
Triple TT(M.getTargetTriple());
if (TT.supportsCOMDAT()) {
@@ -1829,11 +1834,6 @@ static bool InstrumentAllFunctions(
Module &M, function_ref<TargetLibraryInfo &(Function &)> LookupTLI,
function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
function_ref<BlockFrequencyInfo *(Function &)> LookupBFI, bool IsCS) {
- // For the context-sensitve instrumentation, we should have a separated pass
- // (before LTO/ThinLTO linking) to create these variables.
- if (!IsCS && !PGOCtxProfLoweringPass::isContextualIRPGOEnabled())
- createIRLevelProfileFlagVar(M, /*IsCS=*/false);
-
Triple TT(M.getTargetTriple());
LLVMContext &Ctx = M.getContext();
if (!TT.isOSBinFormatELF() && EnableVTableValueProfiling)
@@ -1855,9 +1855,15 @@ static bool InstrumentAllFunctions(
instrumentOneFunc(F, &M, TLI, BPI, BFI, ComdatMembers, IsCS);
AnythingInstrumented = true;
}
+
if (!AnythingInstrumented)
return false;
+ // For the context-sensitve instrumentation, we should have a separated pass
+ // (before LTO/ThinLTO linking) to create these variables.
+ if (!IsCS && !PGOCtxProfLoweringPass::isContextualIRPGOEnabled())
+ createIRLevelProfileFlagVar(M, /*IsCS=*/false);
+
return true;
}
diff --git a/llvm/test/Transforms/PGOProfile/declarations_only.ll b/llvm/test/Transforms/PGOProfile/declarations_only.ll
index e7208fc264c7c..7a975725410c9 100644
--- a/llvm/test/Transforms/PGOProfile/declarations_only.ll
+++ b/llvm/test/Transforms/PGOProfile/declarations_only.ll
@@ -1,13 +1,8 @@
-; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s --check-prefix=GEN --check-prefix=GEN-COMDAT
+; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s --implicit-check-not='__llvm_profile_raw_version'
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
-; GEN-COMDAT: $__llvm_profile_raw_version = comdat any
-; GEN-COMDAT: @__llvm_profile_raw_version = hidden constant i64 {{[0-9]+}}, comdat
-; GEN-NOT: @__profn_test_1 = private constant [6 x i8] c"test_1"
-; GEN-NOT: @__profn_test_2 = private constant [6 x i8] c"test_2"
-
declare i32 @test_1(i32 %i)
declare i32 @test_2(i32 %i)
diff --git a/llvm/test/Transforms/PGOProfile/global_variables_only.ll b/llvm/test/Transforms/PGOProfile/global_variables_only.ll
index 3bfa29af5d34f..46628bfafe534 100644
--- a/llvm/test/Transforms/PGOProfile/global_variables_only.ll
+++ b/llvm/test/Transforms/PGOProfile/global_variables_only.ll
@@ -1,9 +1,6 @@
-; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s --check-prefix=GEN-COMDAT
+; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s --implicit-check-not='__llvm_profile_raw_version'
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
-; GEN-COMDAT: $__llvm_profile_raw_version = comdat any
-; GEN-COMDAT: @__llvm_profile_raw_version = hidden constant i64 {{[0-9]+}}, comdat
-
@var = internal unnamed_addr global [35 x ptr] zeroinitializer, align 16
diff --git a/llvm/test/Transforms/PGOProfile/no_global_variables.ll b/llvm/test/Transforms/PGOProfile/no_global_variables.ll
new file mode 100644
index 0000000000000..0faa2f35274c9
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/no_global_variables.ll
@@ -0,0 +1,15 @@
+; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s --check-prefix=GEN-COMDAT
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+; GEN-COMDAT: $__llvm_profile_raw_version = comdat any
+; GEN-COMDAT: @__llvm_profile_raw_version = hidden constant i64 {{[0-9]+}}, comdat
+
+define i32 @test_1(i32 %i) {
+entry:
+ ret i32 0
+}
+
+define i32 @test_2(i32 %i) {
+entry:
+ ret i32 1
+}
diff --git a/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp b/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp
index cbeaa501d4d88..01d17c5059a1a 100644
--- a/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp
+++ b/llvm/unittests/Transforms/Instrumentation/PGOInstrumentationTest.cpp
@@ -217,8 +217,7 @@ TEST_P(PGOInstrumentationGenIgnoreTest, NotInstrumented) {
const auto *IRInstrVar =
M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
- EXPECT_THAT(IRInstrVar, NotNull());
- EXPECT_FALSE(IRInstrVar->isDeclaration());
+ EXPECT_THAT(IRInstrVar, IsNull());
}
} // end anonymous namespace
More information about the llvm-branch-commits
mailing list