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

Mircea Trofin via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 10 17:07:44 PST 2023


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 <mtrofin at google.com>
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<Instruction *, Instruction *>;
 /// 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<InstrProfiling> {
+class InstrProfilingLoweringPass
+    : public PassInfoMixin<InstrProfilingLoweringPass> {
+  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<const TargetLibraryInfo &(Function &F)> GetTLI);
+};
+class InstrProfiling final {
+public:
+  InstrProfiling(Module &M, const InstrProfOptions &Options,
+                 std::function<const TargetLibraryInfo &(Function &F)> 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<const TargetLibraryInfo &(Function &F)> 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<GlobalVariable *, PerFunctionProfileData> ProfileDataMap;
   /// If runtime relocation is enabled, this maps functions to the load
@@ -64,11 +78,8 @@ class InstrProfiling : public PassInfoMixin<InstrProfiling> {
   std::vector<GlobalValue *> CompilerUsedVars;
   std::vector<GlobalValue *> UsedVars;
   std::vector<GlobalVariable *> ReferencedNames;
-  GlobalVariable *NamesVar;
-  size_t NamesSize;
-
-  // Is this lowering for the context-sensitive instrumentation.
-  bool IsCS;
+  GlobalVariable *NamesVar = nullptr;
+  size_t NamesSize = 0;
 
   // vector of counter load/store pairs to be register promoted.
   std::vector<LoadStorePair> PromotionCandidates;
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index e7f88680655c12..5c6c391049a7b2 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -810,7 +810,7 @@ void PassBuilder::addPGOInstrPasses(ModulePassManager &MPM,
   Options.DoCounterPromotion = true;
   Options.UseBFIInPromotion = IsCS;
   Options.Atomic = AtomicCounterUpdate;
-  MPM.addPass(InstrProfiling(Options, IsCS));
+  MPM.addPass(InstrProfilingLoweringPass(Options, IsCS));
 }
 
 void PassBuilder::addPGOInstrPassesForO0(
@@ -837,7 +837,7 @@ void PassBuilder::addPGOInstrPassesForO0(
   Options.DoCounterPromotion = false;
   Options.UseBFIInPromotion = IsCS;
   Options.Atomic = AtomicCounterUpdate;
-  MPM.addPass(InstrProfiling(Options, IsCS));
+  MPM.addPass(InstrProfilingLoweringPass(Options, IsCS));
 }
 
 static InlineParams getInlineParamsFromOptLevel(OptimizationLevel Level) {
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 7462704ec2df8e..bba1bb1fc838ec 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -77,7 +77,7 @@ MODULE_PASS("inliner-wrapper-no-mandatory-first",
             ModuleInlinerWrapperPass(getInlineParams(), false))
 MODULE_PASS("insert-gcov-profiling", GCOVProfilerPass())
 MODULE_PASS("instrorderfile", InstrOrderFilePass())
-MODULE_PASS("instrprof", InstrProfiling())
+MODULE_PASS("instrprof", InstrProfilingLoweringPass())
 MODULE_PASS("internalize", InternalizePass())
 MODULE_PASS("invalidate<all>", InvalidateAllAnalysesPass())
 MODULE_PASS("iroutliner", IROutlinerPass())
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index d3282779d9f5f3..adb4ffd4c81272 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -407,13 +407,15 @@ enum class ValueProfilingCallType {
 
 } // end anonymous namespace
 
-PreservedAnalyses InstrProfiling::run(Module &M, ModuleAnalysisManager &AM) {
+PreservedAnalyses InstrProfilingLoweringPass::run(Module &M,
+                                                  ModuleAnalysisManager &AM) {
   FunctionAnalysisManager &FAM =
       AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   auto GetTLI = [&FAM](Function &F) -> TargetLibraryInfo & {
     return FAM.getResult<TargetLibraryAnalysis>(F);
   };
-  if (!run(M, GetTLI))
+  InstrProfiling Lowerer(M, Options, GetTLI, IsCS);
+  if (!Lowerer.lower())
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
@@ -535,17 +537,7 @@ static bool containsProfilingIntrinsics(Module &M) {
          containsIntrinsic(llvm::Intrinsic::instrprof_value_profile);
 }
 
-bool InstrProfiling::run(
-    Module &M, std::function<const TargetLibraryInfo &(Function &F)> GetTLI) {
-  this->M = &M;
-  this->GetTLI = std::move(GetTLI);
-  NamesVar = nullptr;
-  NamesSize = 0;
-  ProfileDataMap.clear();
-  CompilerUsedVars.clear();
-  UsedVars.clear();
-  TT = Triple(M.getTargetTriple());
-
+bool InstrProfiling::lower() {
   bool MadeChange = false;
   bool NeedsRuntimeHook = needsRuntimeHookUnconditionally(TT);
   if (NeedsRuntimeHook)
@@ -678,12 +670,12 @@ void InstrProfiling::lowerValueProfileInst(InstrProfValueProfileInst *Ind) {
   Ind->getOperandBundlesAsDefs(OpBundles);
   if (!IsMemOpSize) {
     Value *Args[3] = {Ind->getTargetValue(), DataVar, Builder.getInt32(Index)};
-    Call = Builder.CreateCall(getOrInsertValueProfilingCall(*M, *TLI), Args,
+    Call = Builder.CreateCall(getOrInsertValueProfilingCall(M, *TLI), Args,
                               OpBundles);
   } else {
     Value *Args[3] = {Ind->getTargetValue(), DataVar, Builder.getInt32(Index)};
     Call = Builder.CreateCall(
-        getOrInsertValueProfilingCall(*M, *TLI, ValueProfilingCallType::MemOp),
+        getOrInsertValueProfilingCall(M, *TLI, ValueProfilingCallType::MemOp),
         Args, OpBundles);
   }
   if (auto AK = TLI->getExtAttrForI32Param(false))
@@ -705,18 +697,18 @@ Value *InstrProfiling::getCounterAddress(InstrProfCntrInstBase *I) {
   if (!isRuntimeCounterRelocationEnabled())
     return Addr;
 
-  Type *Int64Ty = Type::getInt64Ty(M->getContext());
+  Type *Int64Ty = Type::getInt64Ty(M.getContext());
   Function *Fn = I->getParent()->getParent();
   LoadInst *&BiasLI = FunctionToProfileBiasMap[Fn];
   if (!BiasLI) {
     IRBuilder<> EntryBuilder(&Fn->getEntryBlock().front());
-    auto *Bias = M->getGlobalVariable(getInstrProfCounterBiasVarName());
+    auto *Bias = M.getGlobalVariable(getInstrProfCounterBiasVarName());
     if (!Bias) {
       // Compiler must define this variable when runtime counter relocation
       // is being used. Runtime has a weak external reference that is used
       // to check whether that's the case or not.
       Bias = new GlobalVariable(
-          *M, Int64Ty, false, GlobalValue::LinkOnceODRLinkage,
+          M, Int64Ty, false, GlobalValue::LinkOnceODRLinkage,
           Constant::getNullValue(Int64Ty), getInstrProfCounterBiasVarName());
       Bias->setVisibility(GlobalVariable::HiddenVisibility);
       // A definition that's weak (linkonce_odr) without being in a COMDAT
@@ -724,7 +716,7 @@ Value *InstrProfiling::getCounterAddress(InstrProfCntrInstBase *I) {
       // data word from every TU but one. Putting it in COMDAT ensures there
       // will be exactly one data slot in the link.
       if (TT.supportsCOMDAT())
-        Bias->setComdat(M->getOrInsertComdat(Bias->getName()));
+        Bias->setComdat(M.getOrInsertComdat(Bias->getName()));
     }
     BiasLI = EntryBuilder.CreateLoad(Int64Ty, Bias);
   }
@@ -740,9 +732,9 @@ Value *InstrProfiling::getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I) {
       Bitmaps->getValueType(), Bitmaps, 0, I->getBitmapIndex()->getZExtValue());
 
   if (isRuntimeCounterRelocationEnabled()) {
-    LLVMContext &Ctx = M->getContext();
+    LLVMContext &Ctx = M.getContext();
     Ctx.diagnose(DiagnosticInfoPGOProfile(
-        M->getName().data(),
+        M.getName().data(),
         Twine("Runtime counter relocation is presently not supported for MC/DC "
               "bitmaps."),
         DS_Warning));
@@ -763,12 +755,12 @@ void InstrProfiling::lowerTimestamp(
     InstrProfTimestampInst *TimestampInstruction) {
   assert(TimestampInstruction->getIndex()->isZeroValue() &&
          "timestamp probes are always the first probe for a function");
-  auto &Ctx = M->getContext();
+  auto &Ctx = M.getContext();
   auto *TimestampAddr = getCounterAddress(TimestampInstruction);
   IRBuilder<> Builder(TimestampInstruction);
   auto *CalleeTy =
       FunctionType::get(Type::getVoidTy(Ctx), TimestampAddr->getType(), false);
-  auto Callee = M->getOrInsertFunction(
+  auto Callee = M.getOrInsertFunction(
       INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_SET_TIMESTAMP), CalleeTy);
   Builder.CreateCall(Callee, {TimestampAddr});
   TimestampInstruction->eraseFromParent();
@@ -813,10 +805,10 @@ void InstrProfiling::lowerCoverageData(GlobalVariable *CoverageNamesVar) {
 void InstrProfiling::lowerMCDCTestVectorBitmapUpdate(
     InstrProfMCDCTVBitmapUpdate *Update) {
   IRBuilder<> Builder(Update);
-  auto *Int8Ty = Type::getInt8Ty(M->getContext());
-  auto *Int8PtrTy = PointerType::getUnqual(M->getContext());
-  auto *Int32Ty = Type::getInt32Ty(M->getContext());
-  auto *Int64Ty = Type::getInt64Ty(M->getContext());
+  auto *Int8Ty = Type::getInt8Ty(M.getContext());
+  auto *Int8PtrTy = PointerType::getUnqual(M.getContext());
+  auto *Int32Ty = Type::getInt32Ty(M.getContext());
+  auto *Int64Ty = Type::getInt64Ty(M.getContext());
   auto *MCDCCondBitmapAddr = Update->getMCDCCondBitmapAddr();
   auto *BitmapAddr = getBitmapAddress(Update);
 
@@ -865,7 +857,7 @@ void InstrProfiling::lowerMCDCTestVectorBitmapUpdate(
 void InstrProfiling::lowerMCDCCondBitmapUpdate(
     InstrProfMCDCCondBitmapUpdate *Update) {
   IRBuilder<> Builder(Update);
-  auto *Int32Ty = Type::getInt32Ty(M->getContext());
+  auto *Int32Ty = Type::getInt32Ty(M.getContext());
   auto *MCDCCondBitmapAddr = Update->getMCDCCondBitmapAddr();
 
   // Load the MCDC temporary value from the stack.
@@ -1047,8 +1039,8 @@ static bool needsRuntimeRegistrationOfSectionRange(const Triple &TT) {
 
 void InstrProfiling::maybeSetComdat(GlobalVariable *GV, Function *Fn,
                                     StringRef VarName) {
-  bool DataReferencedByCode = profDataReferencedByCode(*M);
-  bool NeedComdat = needsComdatForCounter(*Fn, *M);
+  bool DataReferencedByCode = profDataReferencedByCode(M);
+  bool NeedComdat = needsComdatForCounter(*Fn, M);
   bool UseComdat = (NeedComdat || TT.isOSBinFormatELF());
 
   if (!UseComdat)
@@ -1056,7 +1048,7 @@ void InstrProfiling::maybeSetComdat(GlobalVariable *GV, Function *Fn,
 
   StringRef GroupName =
       TT.isOSBinFormatCOFF() && DataReferencedByCode ? GV->getName() : VarName;
-  Comdat *C = M->getOrInsertComdat(GroupName);
+  Comdat *C = M.getOrInsertComdat(GroupName);
   if (!NeedComdat)
     C->setSelectionKind(Comdat::NoDeduplicate);
   GV->setComdat(C);
@@ -1141,8 +1133,8 @@ InstrProfiling::createRegionBitmaps(InstrProfMCDCBitmapInstBase *Inc,
                                     StringRef Name,
                                     GlobalValue::LinkageTypes Linkage) {
   uint64_t NumBytes = Inc->getNumBitmapBytes()->getZExtValue();
-  auto *BitmapTy = ArrayType::get(Type::getInt8Ty(M->getContext()), NumBytes);
-  auto GV = new GlobalVariable(*M, BitmapTy, false, Linkage,
+  auto *BitmapTy = ArrayType::get(Type::getInt8Ty(M.getContext()), NumBytes);
+  auto GV = new GlobalVariable(M, BitmapTy, false, Linkage,
                                Constant::getNullValue(BitmapTy), Name);
   GV->setAlignment(Align(1));
   return GV;
@@ -1167,7 +1159,7 @@ GlobalVariable *
 InstrProfiling::createRegionCounters(InstrProfCntrInstBase *Inc, StringRef Name,
                                      GlobalValue::LinkageTypes Linkage) {
   uint64_t NumCounters = Inc->getNumCounters()->getZExtValue();
-  auto &Ctx = M->getContext();
+  auto &Ctx = M.getContext();
   GlobalVariable *GV;
   if (isa<InstrProfCoverInst>(Inc)) {
     auto *CounterTy = Type::getInt8Ty(Ctx);
@@ -1175,13 +1167,13 @@ InstrProfiling::createRegionCounters(InstrProfCntrInstBase *Inc, StringRef Name,
     // TODO: `Constant::getAllOnesValue()` does not yet accept an array type.
     std::vector<Constant *> InitialValues(NumCounters,
                                           Constant::getAllOnesValue(CounterTy));
-    GV = new GlobalVariable(*M, CounterArrTy, false, Linkage,
+    GV = new GlobalVariable(M, CounterArrTy, false, Linkage,
                             ConstantArray::get(CounterArrTy, InitialValues),
                             Name);
     GV->setAlignment(Align(1));
   } else {
     auto *CounterTy = ArrayType::get(Type::getInt64Ty(Ctx), NumCounters);
-    GV = new GlobalVariable(*M, CounterTy, false, Linkage,
+    GV = new GlobalVariable(M, CounterTy, false, Linkage,
                             Constant::getNullValue(CounterTy), Name);
     GV->setAlignment(Align(8));
   }
@@ -1201,10 +1193,10 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfCntrInstBase *Inc) {
   PD.RegionCounters = CounterPtr;
 
   if (DebugInfoCorrelate) {
-    LLVMContext &Ctx = M->getContext();
+    LLVMContext &Ctx = M.getContext();
     Function *Fn = Inc->getParent()->getParent();
     if (auto *SP = Fn->getSubprogram()) {
-      DIBuilder DB(*M, true, SP->getUnit());
+      DIBuilder DB(M, true, SP->getUnit());
       Metadata *FunctionNameAnnotation[] = {
           MDString::get(Ctx, InstrProfCorrelator::FunctionNameAttributeName),
           MDString::get(Ctx, getPGOFuncNameVarInitializer(NamePtr)),
@@ -1255,7 +1247,7 @@ void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc) {
   if (PD.DataVar)
     return;
 
-  LLVMContext &Ctx = M->getContext();
+  LLVMContext &Ctx = M.getContext();
 
   Function *Fn = Inc->getParent()->getParent();
   GlobalValue::LinkageTypes Linkage = NamePtr->getLinkage();
@@ -1271,8 +1263,8 @@ void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc) {
     Visibility = GlobalValue::DefaultVisibility;
   }
 
-  bool DataReferencedByCode = profDataReferencedByCode(*M);
-  bool NeedComdat = needsComdatForCounter(*Fn, *M);
+  bool DataReferencedByCode = profDataReferencedByCode(M);
+  bool NeedComdat = needsComdatForCounter(*Fn, M);
   bool Renamed;
 
   // The Data Variable section is anchored to profile counters.
@@ -1292,7 +1284,7 @@ void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc) {
       !needsRuntimeRegistrationOfSectionRange(TT)) {
     ArrayType *ValuesTy = ArrayType::get(Type::getInt64Ty(Ctx), NS);
     auto *ValuesVar = new GlobalVariable(
-        *M, ValuesTy, false, Linkage, Constant::getNullValue(ValuesTy),
+        M, ValuesTy, false, Linkage, Constant::getNullValue(ValuesTy),
         getVarName(Inc, getInstrProfValuesVarPrefix(), Renamed));
     ValuesVar->setVisibility(Visibility);
     setGlobalVariableLargeSection(TT, *ValuesVar);
@@ -1309,7 +1301,7 @@ void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc) {
   uint64_t NumBitmapBytes = PD.NumBitmapBytes;
 
   // Create data variable.
-  auto *IntPtrTy = M->getDataLayout().getIntPtrType(M->getContext());
+  auto *IntPtrTy = M.getDataLayout().getIntPtrType(M.getContext());
   auto *Int16Ty = Type::getInt16Ty(Ctx);
   auto *Int16ArrayTy = ArrayType::get(Int16Ty, IPVK_Last + 1);
   Type *DataTypes[] = {
@@ -1342,7 +1334,7 @@ void InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc) {
     Visibility = GlobalValue::DefaultVisibility;
   }
   auto *Data =
-      new GlobalVariable(*M, DataTy, false, Linkage, nullptr, DataVarName);
+      new GlobalVariable(M, DataTy, false, Linkage, nullptr, DataVarName);
   // Reference the counter variable with a label difference (link-time
   // constant).
   auto *RelativeCounterPtr =
@@ -1413,7 +1405,7 @@ void InstrProfiling::emitVNodes() {
   if (NumCounters < INSTR_PROF_MIN_VAL_COUNTS)
     NumCounters = std::max(INSTR_PROF_MIN_VAL_COUNTS, (int)NumCounters * 2);
 
-  auto &Ctx = M->getContext();
+  auto &Ctx = M.getContext();
   Type *VNodeTypes[] = {
 #define INSTR_PROF_VALUE_NODE(Type, LLVMType, Name, Init) LLVMType,
 #include "llvm/ProfileData/InstrProfData.inc"
@@ -1422,12 +1414,12 @@ void InstrProfiling::emitVNodes() {
 
   ArrayType *VNodesTy = ArrayType::get(VNodeTy, NumCounters);
   auto *VNodesVar = new GlobalVariable(
-      *M, VNodesTy, false, GlobalValue::PrivateLinkage,
+      M, VNodesTy, false, GlobalValue::PrivateLinkage,
       Constant::getNullValue(VNodesTy), getInstrProfVNodesVarName());
   setGlobalVariableLargeSection(TT, *VNodesVar);
   VNodesVar->setSection(
       getInstrProfSectionName(IPSK_vnodes, TT.getObjectFormat()));
-  VNodesVar->setAlignment(M->getDataLayout().getABITypeAlign(VNodesTy));
+  VNodesVar->setAlignment(M.getDataLayout().getABITypeAlign(VNodesTy));
   // VNodesVar is used by runtime but not referenced via relocation by other
   // sections. Conservatively make it linker retained.
   UsedVars.push_back(VNodesVar);
@@ -1445,10 +1437,10 @@ void InstrProfiling::emitNameData() {
     report_fatal_error(Twine(toString(std::move(E))), false);
   }
 
-  auto &Ctx = M->getContext();
+  auto &Ctx = M.getContext();
   auto *NamesVal =
       ConstantDataArray::getString(Ctx, StringRef(CompressedNameStr), false);
-  NamesVar = new GlobalVariable(*M, NamesVal->getType(), true,
+  NamesVar = new GlobalVariable(M, NamesVal->getType(), true,
                                 GlobalValue::PrivateLinkage, NamesVal,
                                 getInstrProfNamesVarName());
   NamesSize = CompressedNameStr.size();
@@ -1472,9 +1464,9 @@ void InstrProfiling::emitRegistration() {
     return;
 
   // Construct the function.
-  auto *VoidTy = Type::getVoidTy(M->getContext());
-  auto *VoidPtrTy = PointerType::getUnqual(M->getContext());
-  auto *Int64Ty = Type::getInt64Ty(M->getContext());
+  auto *VoidTy = Type::getVoidTy(M.getContext());
+  auto *VoidPtrTy = PointerType::getUnqual(M.getContext());
+  auto *Int64Ty = Type::getInt64Ty(M.getContext());
   auto *RegisterFTy = FunctionType::get(VoidTy, false);
   auto *RegisterF = Function::Create(RegisterFTy, GlobalValue::InternalLinkage,
                                      getInstrProfRegFuncsName(), M);
@@ -1487,7 +1479,7 @@ void InstrProfiling::emitRegistration() {
       Function::Create(RuntimeRegisterTy, GlobalVariable::ExternalLinkage,
                        getInstrProfRegFuncName(), M);
 
-  IRBuilder<> IRB(BasicBlock::Create(M->getContext(), "", RegisterF));
+  IRBuilder<> IRB(BasicBlock::Create(M.getContext(), "", RegisterF));
   for (Value *Data : CompilerUsedVars)
     if (!isa<Function>(Data))
       IRB.CreateCall(RuntimeRegisterF, Data);
@@ -1515,13 +1507,13 @@ bool InstrProfiling::emitRuntimeHook() {
     return false;
 
   // If the module's provided its own runtime, we don't need to do anything.
-  if (M->getGlobalVariable(getInstrProfRuntimeHookVarName()))
+  if (M.getGlobalVariable(getInstrProfRuntimeHookVarName()))
     return false;
 
   // Declare an external variable that will pull in the runtime initialization.
-  auto *Int32Ty = Type::getInt32Ty(M->getContext());
+  auto *Int32Ty = Type::getInt32Ty(M.getContext());
   auto *Var =
-      new GlobalVariable(*M, Int32Ty, false, GlobalValue::ExternalLinkage,
+      new GlobalVariable(M, Int32Ty, false, GlobalValue::ExternalLinkage,
                          nullptr, getInstrProfRuntimeHookVarName());
   Var->setVisibility(GlobalValue::HiddenVisibility);
 
@@ -1538,9 +1530,9 @@ bool InstrProfiling::emitRuntimeHook() {
       User->addFnAttr(Attribute::NoRedZone);
     User->setVisibility(GlobalValue::HiddenVisibility);
     if (TT.supportsCOMDAT())
-      User->setComdat(M->getOrInsertComdat(User->getName()));
+      User->setComdat(M.getOrInsertComdat(User->getName()));
 
-    IRBuilder<> IRB(BasicBlock::Create(M->getContext(), "", User));
+    IRBuilder<> IRB(BasicBlock::Create(M.getContext(), "", User));
     auto *Load = IRB.CreateLoad(Int32Ty, Var);
     IRB.CreateRet(Load);
 
@@ -1561,15 +1553,15 @@ void InstrProfiling::emitUses() {
   // and ensure this GC property as well. Otherwise, we have to conservatively
   // make all of the sections retained by the linker.
   if (TT.isOSBinFormatELF() || TT.isOSBinFormatMachO() ||
-      (TT.isOSBinFormatCOFF() && !profDataReferencedByCode(*M)))
-    appendToCompilerUsed(*M, CompilerUsedVars);
+      (TT.isOSBinFormatCOFF() && !profDataReferencedByCode(M)))
+    appendToCompilerUsed(M, CompilerUsedVars);
   else
-    appendToUsed(*M, CompilerUsedVars);
+    appendToUsed(M, CompilerUsedVars);
 
   // We do not add proper references from used metadata sections to NamesVar and
   // VNodesVar, so we have to be conservative and place them in llvm.used
   // regardless of the target,
-  appendToUsed(*M, UsedVars);
+  appendToUsed(M, UsedVars);
 }
 
 void InstrProfiling::emitInitialization() {
@@ -1578,13 +1570,13 @@ void InstrProfiling::emitInitialization() {
   // LTO/ThinLTO linking. Pass PGOInstrumentationGenCreateVar should
   // have already create the variable before LTO/ThinLTO linking.
   if (!IsCS)
-    createProfileFileNameVar(*M, Options.InstrProfileOutput);
-  Function *RegisterF = M->getFunction(getInstrProfRegFuncsName());
+    createProfileFileNameVar(M, Options.InstrProfileOutput);
+  Function *RegisterF = M.getFunction(getInstrProfRegFuncsName());
   if (!RegisterF)
     return;
 
   // Create the initialization function.
-  auto *VoidTy = Type::getVoidTy(M->getContext());
+  auto *VoidTy = Type::getVoidTy(M.getContext());
   auto *F = Function::Create(FunctionType::get(VoidTy, false),
                              GlobalValue::InternalLinkage,
                              getInstrProfInitFuncName(), M);
@@ -1594,9 +1586,9 @@ void InstrProfiling::emitInitialization() {
     F->addFnAttr(Attribute::NoRedZone);
 
   // Add the basic block and the necessary calls.
-  IRBuilder<> IRB(BasicBlock::Create(M->getContext(), "", F));
+  IRBuilder<> IRB(BasicBlock::Create(M.getContext(), "", F));
   IRB.CreateCall(RegisterF, {});
   IRB.CreateRetVoid();
 
-  appendToGlobalCtors(*M, F, 0);
+  appendToGlobalCtors(M, F, 0);
 }
diff --git a/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp b/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
index 199afbe966dde4..7a03ee46d6fded 100644
--- a/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
@@ -85,7 +85,7 @@ Comdat *llvm::getOrCreateFunctionComdat(Function &F, Triple &T) {
   return C;
 }
 
-void llvm::setGlobalVariableLargeSection(Triple &TargetTriple,
+void llvm::setGlobalVariableLargeSection(const Triple &TargetTriple,
                                          GlobalVariable &GV) {
   if (TargetTriple.getArch() == Triple::x86_64 &&
       TargetTriple.getObjectFormat() == Triple::ELF) {

>From 60def86fb9a175a9016356277c45a397b12ebd36 Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Sun, 10 Dec 2023 17:07:13 -0800
Subject: [PATCH 2/2] feedback; also fixed clang-side test

---
 clang/test/CodeGen/pgo-instrumentation.c                     | 4 ++--
 .../include/llvm/Transforms/Instrumentation/InstrProfiling.h | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/clang/test/CodeGen/pgo-instrumentation.c b/clang/test/CodeGen/pgo-instrumentation.c
index a65c6712291bd2..c01658065497e3 100644
--- a/clang/test/CodeGen/pgo-instrumentation.c
+++ b/clang/test/CodeGen/pgo-instrumentation.c
@@ -3,7 +3,7 @@
 // Ensure Pass PGOInstrumentationGenPass is invoked.
 // RUN: %clang_cc1 -O2 -fprofile-instrument=llvm %s  -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN --check-prefix=CHECK-INSTRPROF
 // CHECK-PGOGENPASS-INVOKED-INSTR-GEN: Running pass: PGOInstrumentationGen on
-// CHECK-INSTRPROF: Running pass: InstrProfiling on
+// CHECK-INSTRPROF: Running pass: InstrProfilingLoweringPass on
 //
 // Ensure Pass PGOInstrumentationGenPass is not invoked.
 // RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG
@@ -11,7 +11,7 @@
 
 // RUN: %clang_cc1 -O2 -fprofile-instrument=clang %s -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF
 // RUN: %clang_cc1 -O0 -fprofile-instrument=clang %s -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-CLANG-INSTRPROF
-// CHECK-CLANG-INSTRPROF: Running pass: InstrProfiling on
+// CHECK-CLANG-INSTRPROF: Running pass: InstrProfilingLoweringPass on
 
 // Ensure Pass PGOInstrumentationUsePass is invoked.
 // RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgotestir.profraw
diff --git a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
index 0d778288d78455..89fa08685692b7 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
@@ -35,15 +35,16 @@ class InstrProfilingLoweringPass
     : public PassInfoMixin<InstrProfilingLoweringPass> {
   const InstrProfOptions Options;
   // Is this lowering for the context-sensitive instrumentation.
-  const bool IsCS;
+  const bool IsCS = false;
 
 public:
-  InstrProfilingLoweringPass() : IsCS(false) {}
+  InstrProfilingLoweringPass() = default;
   InstrProfilingLoweringPass(const InstrProfOptions &Options, bool IsCS = false)
       : Options(Options), IsCS(IsCS) {}
 
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 };
+
 class InstrProfiling final {
 public:
   InstrProfiling(Module &M, const InstrProfOptions &Options,



More information about the cfe-commits mailing list