[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
https://github.com/mtrofin updated
https://github.com/llvm/llvm-project/pull/74970
>From 90893a3b3f71524947cb041b2a25d0a02a8956d7 Mon Sep 17 00:00:00 2001
From: Mircea Trofin
Date: Sat, 9 Dec 2023 21:24:35 -0800
Subject: [PATCH 1/2] [NFC][InstrProf] Refactor InstrProfiling lowering pass
Akin other passes - refactored the name to `InstrProfilingLoweringPass`
to better communicate what it does, and split the pass part and the
transformation part to avoid needing to initialize object state during
`::run`.
---
clang/lib/CodeGen/BackendUtil.cpp | 2 +-
.../include/llvm/Transforms/Instrumentation.h | 3 +-
.../Instrumentation/InstrProfiling.h | 45 ---
llvm/lib/Passes/PassBuilderPipelines.cpp | 4 +-
llvm/lib/Passes/PassRegistry.def | 2 +-
.../Instrumentation/InstrProfiling.cpp| 126 --
.../Instrumentation/Instrumentation.cpp | 2 +-
7 files changed, 94 insertions(+), 90 deletions(-)
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 8c666e2cb463c6..77455c075cab0d 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -982,7 +982,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
getInstrProfOptions(CodeGenOpts, LangOpts))
PB.registerPipelineStartEPCallback(
[Options](ModulePassManager &MPM, OptimizationLevel Level) {
-MPM.addPass(InstrProfiling(*Options, false));
+MPM.addPass(InstrProfilingLoweringPass(*Options, false));
});
// TODO: Consider passing the MemoryProfileOutput to the pass builder via
diff --git a/llvm/include/llvm/Transforms/Instrumentation.h
b/llvm/include/llvm/Transforms/Instrumentation.h
index bb1a0d446aa2ab..ea97ab2562a5b0 100644
--- a/llvm/include/llvm/Transforms/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Instrumentation.h
@@ -52,7 +52,8 @@ Comdat *getOrCreateFunctionComdat(Function &F, Triple &T);
// Place global in a large section for x86-64 ELF binaries to mitigate
// relocation overflow pressure. This can be be used for metadata globals that
// aren't directly accessed by code, which has no performance impact.
-void setGlobalVariableLargeSection(Triple &TargetTriple, GlobalVariable &GV);
+void setGlobalVariableLargeSection(const Triple &TargetTriple,
+ GlobalVariable &GV);
// Insert GCOV profiling instrumentation
struct GCOVOptions {
diff --git a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
index c106e1651e8045..0d778288d78455 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
@@ -31,31 +31,45 @@ using LoadStorePair = std::pair;
/// Instrumentation based profiling lowering pass. This pass lowers
/// the profile instrumented code generated by FE or the IR based
/// instrumentation pass.
-class InstrProfiling : public PassInfoMixin {
+class InstrProfilingLoweringPass
+: public PassInfoMixin {
+ const InstrProfOptions Options;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
public:
- InstrProfiling() : IsCS(false) {}
- InstrProfiling(const InstrProfOptions &Options, bool IsCS = false)
+ InstrProfilingLoweringPass() : IsCS(false) {}
+ InstrProfilingLoweringPass(const InstrProfOptions &Options, bool IsCS =
false)
: Options(Options), IsCS(IsCS) {}
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
- bool run(Module &M,
- std::function GetTLI);
+};
+class InstrProfiling final {
+public:
+ InstrProfiling(Module &M, const InstrProfOptions &Options,
+ std::function GetTLI,
+ bool IsCS)
+ : M(M), Options(Options), TT(Triple(M.getTargetTriple())), IsCS(IsCS),
+GetTLI(GetTLI) {}
+
+ bool lower();
private:
- InstrProfOptions Options;
- Module *M;
- Triple TT;
+ Module &M;
+ const InstrProfOptions Options;
+ const Triple TT;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
std::function GetTLI;
struct PerFunctionProfileData {
-uint32_t NumValueSites[IPVK_Last + 1];
+uint32_t NumValueSites[IPVK_Last + 1] = {};
GlobalVariable *RegionCounters = nullptr;
GlobalVariable *DataVar = nullptr;
GlobalVariable *RegionBitmaps = nullptr;
uint32_t NumBitmapBytes = 0;
-PerFunctionProfileData() {
- memset(NumValueSites, 0, sizeof(uint32_t) * (IPVK_Last + 1));
-}
+PerFunctionProfileData() = default;
};
DenseMap ProfileDataMap;
/// If runtime relocation is enabled, this maps functions to the load
@@ -64,11 +78,8 @@ class InstrProfiling : public PassInfoMixin {
std::vector CompilerUsedVars;
std::vector UsedVars;
std::vector ReferencedNames;
- GlobalVariable *NamesVar;
- size_t NamesSize;
-
- // Is this lowering
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
@@ -31,31 +31,45 @@ using LoadStorePair = std::pair;
/// Instrumentation based profiling lowering pass. This pass lowers
/// the profile instrumented code generated by FE or the IR based
/// instrumentation pass.
-class InstrProfiling : public PassInfoMixin {
+class InstrProfilingLoweringPass
+: public PassInfoMixin {
+ const InstrProfOptions Options;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
public:
- InstrProfiling() : IsCS(false) {}
- InstrProfiling(const InstrProfOptions &Options, bool IsCS = false)
+ InstrProfilingLoweringPass() : IsCS(false) {}
+ InstrProfilingLoweringPass(const InstrProfOptions &Options, bool IsCS =
false)
: Options(Options), IsCS(IsCS) {}
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
- bool run(Module &M,
- std::function GetTLI);
+};
+class InstrProfiling final {
mtrofin wrote:
Done, will move it to the cpp next.
https://github.com/llvm/llvm-project/pull/74970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
@@ -31,31 +31,45 @@ using LoadStorePair = std::pair;
/// Instrumentation based profiling lowering pass. This pass lowers
/// the profile instrumented code generated by FE or the IR based
/// instrumentation pass.
-class InstrProfiling : public PassInfoMixin {
+class InstrProfilingLoweringPass
+: public PassInfoMixin {
+ const InstrProfOptions Options;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
kazutakahirata wrote:
Could we say `const bool IsCS = false;`here to enable the defaulted default
constructor below?
https://github.com/llvm/llvm-project/pull/74970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
@@ -31,31 +31,45 @@ using LoadStorePair = std::pair;
/// Instrumentation based profiling lowering pass. This pass lowers
/// the profile instrumented code generated by FE or the IR based
/// instrumentation pass.
-class InstrProfiling : public PassInfoMixin {
+class InstrProfilingLoweringPass
+: public PassInfoMixin {
+ const InstrProfOptions Options;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
public:
- InstrProfiling() : IsCS(false) {}
- InstrProfiling(const InstrProfOptions &Options, bool IsCS = false)
+ InstrProfilingLoweringPass() : IsCS(false) {}
kazutakahirata wrote:
We could "move" `false` to the declaration above:
`InstrProfilingLoweringPass() = default;`
https://github.com/llvm/llvm-project/pull/74970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
https://github.com/kazutakahirata edited https://github.com/llvm/llvm-project/pull/74970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
@@ -31,31 +31,45 @@ using LoadStorePair = std::pair;
/// Instrumentation based profiling lowering pass. This pass lowers
/// the profile instrumented code generated by FE or the IR based
/// instrumentation pass.
-class InstrProfiling : public PassInfoMixin {
+class InstrProfilingLoweringPass
+: public PassInfoMixin {
+ const InstrProfOptions Options;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
public:
- InstrProfiling() : IsCS(false) {}
- InstrProfiling(const InstrProfOptions &Options, bool IsCS = false)
+ InstrProfilingLoweringPass() : IsCS(false) {}
+ InstrProfilingLoweringPass(const InstrProfOptions &Options, bool IsCS =
false)
: Options(Options), IsCS(IsCS) {}
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
- bool run(Module &M,
- std::function GetTLI);
+};
+class InstrProfiling final {
kazutakahirata wrote:
May I suggest an empty line just before `class InstrProfiling final {`?
https://github.com/llvm/llvm-project/pull/74970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
https://github.com/mtrofin edited https://github.com/llvm/llvm-project/pull/74970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang
Author: Mircea Trofin (mtrofin)
Changes
Akin other passes - refactored the name to `InstrProfilingLoweringPass` to
better communicate what it does, and split the pass part and the transformation
part to avoid needing to initialize object state during `::run`.
---
Patch is 25.04 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/74970.diff
7 Files Affected:
- (modified) clang/lib/CodeGen/BackendUtil.cpp (+1-1)
- (modified) llvm/include/llvm/Transforms/Instrumentation.h (+2-1)
- (modified) llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
(+28-17)
- (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+2-2)
- (modified) llvm/lib/Passes/PassRegistry.def (+1-1)
- (modified) llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (+59-67)
- (modified) llvm/lib/Transforms/Instrumentation/Instrumentation.cpp (+1-1)
``diff
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 8c666e2cb463c6..77455c075cab0d 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -982,7 +982,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
getInstrProfOptions(CodeGenOpts, LangOpts))
PB.registerPipelineStartEPCallback(
[Options](ModulePassManager &MPM, OptimizationLevel Level) {
-MPM.addPass(InstrProfiling(*Options, false));
+MPM.addPass(InstrProfilingLoweringPass(*Options, false));
});
// TODO: Consider passing the MemoryProfileOutput to the pass builder via
diff --git a/llvm/include/llvm/Transforms/Instrumentation.h
b/llvm/include/llvm/Transforms/Instrumentation.h
index bb1a0d446aa2ab..ea97ab2562a5b0 100644
--- a/llvm/include/llvm/Transforms/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Instrumentation.h
@@ -52,7 +52,8 @@ Comdat *getOrCreateFunctionComdat(Function &F, Triple &T);
// Place global in a large section for x86-64 ELF binaries to mitigate
// relocation overflow pressure. This can be be used for metadata globals that
// aren't directly accessed by code, which has no performance impact.
-void setGlobalVariableLargeSection(Triple &TargetTriple, GlobalVariable &GV);
+void setGlobalVariableLargeSection(const Triple &TargetTriple,
+ GlobalVariable &GV);
// Insert GCOV profiling instrumentation
struct GCOVOptions {
diff --git a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
index c106e1651e8045..0d778288d78455 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
@@ -31,31 +31,45 @@ using LoadStorePair = std::pair;
/// Instrumentation based profiling lowering pass. This pass lowers
/// the profile instrumented code generated by FE or the IR based
/// instrumentation pass.
-class InstrProfiling : public PassInfoMixin {
+class InstrProfilingLoweringPass
+: public PassInfoMixin {
+ const InstrProfOptions Options;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
public:
- InstrProfiling() : IsCS(false) {}
- InstrProfiling(const InstrProfOptions &Options, bool IsCS = false)
+ InstrProfilingLoweringPass() : IsCS(false) {}
+ InstrProfilingLoweringPass(const InstrProfOptions &Options, bool IsCS =
false)
: Options(Options), IsCS(IsCS) {}
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
- bool run(Module &M,
- std::function GetTLI);
+};
+class InstrProfiling final {
+public:
+ InstrProfiling(Module &M, const InstrProfOptions &Options,
+ std::function GetTLI,
+ bool IsCS)
+ : M(M), Options(Options), TT(Triple(M.getTargetTriple())), IsCS(IsCS),
+GetTLI(GetTLI) {}
+
+ bool lower();
private:
- InstrProfOptions Options;
- Module *M;
- Triple TT;
+ Module &M;
+ const InstrProfOptions Options;
+ const Triple TT;
+ // Is this lowering for the context-sensitive instrumentation.
+ const bool IsCS;
+
std::function GetTLI;
struct PerFunctionProfileData {
-uint32_t NumValueSites[IPVK_Last + 1];
+uint32_t NumValueSites[IPVK_Last + 1] = {};
GlobalVariable *RegionCounters = nullptr;
GlobalVariable *DataVar = nullptr;
GlobalVariable *RegionBitmaps = nullptr;
uint32_t NumBitmapBytes = 0;
-PerFunctionProfileData() {
- memset(NumValueSites, 0, sizeof(uint32_t) * (IPVK_Last + 1));
-}
+PerFunctionProfileData() = default;
};
DenseMap ProfileDataMap;
/// If runtime relocation is enabled, this maps functions to the load
@@ -64,11 +78,8 @@ class InstrProfiling : public PassInfoMixin {
std::vector CompilerUsedVars;
std::vector UsedVars;
std::vector ReferencedNames;
- GlobalVariable *NamesVar;
- size_t NamesSize;
-
- // Is this l
