[llvm] [clang] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)

2023-12-10 Thread Mircea Trofin via cfe-commits

https://github.com/mtrofin closed 
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


[llvm] [clang] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)

2023-12-10 Thread Kazu Hirata via cfe-commits

https://github.com/kazutakahirata approved this pull request.

LGTM module stylistic comments.

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


[llvm] [clang] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)

2023-12-09 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-llvm-transforms

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 , 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 , Triple );
 // 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 , GlobalVariable );
+void setGlobalVariableLargeSection(const Triple ,
+   GlobalVariable );
 
 // 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 , bool IsCS = false)
+  InstrProfilingLoweringPass() : IsCS(false) {}
+  InstrProfilingLoweringPass(const InstrProfOptions , bool IsCS = 
false)
   : Options(Options), IsCS(IsCS) {}
 
   PreservedAnalyses run(Module , ModuleAnalysisManager );
-  bool run(Module ,
-   std::function GetTLI);
+};
+class InstrProfiling final {
+public:
+  InstrProfiling(Module , const InstrProfOptions ,
+ 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 
+  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 for the context-sensitive instrumentation.
-  bool IsCS;
+  GlobalVariable *NamesVar = 

[llvm] [clang] [NFC][InstrProf] Refactor InstrProfiling lowering pass (PR #74970)

2023-12-09 Thread Mircea Trofin via cfe-commits

https://github.com/mtrofin created 
https://github.com/llvm/llvm-project/pull/74970

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`.

>From 90893a3b3f71524947cb041b2a25d0a02a8956d7 Mon Sep 17 00:00:00 2001
From: Mircea Trofin 
Date: Sat, 9 Dec 2023 21:24:35 -0800
Subject: [PATCH] [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 , 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 , Triple );
 // 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 , GlobalVariable );
+void setGlobalVariableLargeSection(const Triple ,
+   GlobalVariable );
 
 // 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 , bool IsCS = false)
+  InstrProfilingLoweringPass() : IsCS(false) {}
+  InstrProfilingLoweringPass(const InstrProfOptions , bool IsCS = 
false)
   : Options(Options), IsCS(IsCS) {}
 
   PreservedAnalyses run(Module , ModuleAnalysisManager );
-  bool run(Module ,
-   std::function GetTLI);
+};
+class InstrProfiling final {
+public:
+  InstrProfiling(Module , const InstrProfOptions ,
+ 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 
+  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