[NFC][InstrProf] Refactor InstrProfiling lowering pass #74970

mtrofin · 2023-12-10T05:38:30Z

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.

A subsequent PR will move InstrLowering to the .cpp file and rename it to InstrLowerer.

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

llvmbot · 2023-12-10T05:38:56Z

@llvm/pr-subscribers-pgo
@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 --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.
-...
[truncated]

llvmbot · 2023-12-10T05:38:56Z

@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 --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.
-...
[truncated]

kazutakahirata

LGTM module stylistic comments.

llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h

[NFC][InstrProf] Refactor InstrProfiling lowering pass

90893a3

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

mtrofin requested review from david-xl, kazutakahirata and xur-llvm December 10, 2023 05:38

llvmbot added clang Clang issues not falling into any other category clang:codegen PGO Profile Guided Optimizations llvm:transforms labels Dec 10, 2023

kazutakahirata approved these changes Dec 10, 2023

View reviewed changes

llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h Outdated Show resolved Hide resolved

llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h Outdated Show resolved Hide resolved

llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h Show resolved Hide resolved

feedback; also fixed clang-side test

60def86

mtrofin merged commit 1d608fc into llvm:main Dec 11, 2023
4 checks passed

mtrofin deleted the pgolower branch December 11, 2023 02:03

mtrofin added a commit that referenced this pull request Dec 11, 2023

Fix uninitialized field post PR #74970

a52ac7f

nico mentioned this pull request Dec 11, 2023

llvm-wrapper: adapt for LLVM API change rust-lang/rust#118818

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NFC][InstrProf] Refactor InstrProfiling lowering pass #74970

[NFC][InstrProf] Refactor InstrProfiling lowering pass #74970

mtrofin commented Dec 10, 2023 •

edited

Loading

llvmbot commented Dec 10, 2023 •

edited

Loading

llvmbot commented Dec 10, 2023

kazutakahirata left a comment

[NFC][InstrProf] Refactor InstrProfiling lowering pass #74970

[NFC][InstrProf] Refactor InstrProfiling lowering pass #74970

Conversation

mtrofin commented Dec 10, 2023 • edited Loading

llvmbot commented Dec 10, 2023 • edited Loading

llvmbot commented Dec 10, 2023

kazutakahirata left a comment

Choose a reason for hiding this comment

mtrofin commented Dec 10, 2023 •

edited

Loading

llvmbot commented Dec 10, 2023 •

edited

Loading