Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ctx_prof] Remove the dependency on the "name" GlobalVariable #105731

Merged
merged 1 commit into from
Aug 23, 2024
Merged

[ctx_prof] Remove the dependency on the "name" GlobalVariable #105731

merged 1 commit into from
Aug 23, 2024

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Aug 22, 2024

We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.

@mtrofin Graphite App
Copy link
Member Author

mtrofin commented Aug 22, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mtrofin and the rest of your teammates on Graphite Graphite

@mtrofin mtrofin force-pushed the users/mtrofin/08-22-_ctx_prof_remove_the_dependency_on_the_name_globalvariable branch from 141f202 to f9f3d08 Compare August 22, 2024 20:50
@mtrofin mtrofin requested review from ellishg and xur-llvm August 22, 2024 20:50
@boomanaiden154
Copy link
Contributor

Is this ready to review (still marked as draft)? Also, merge conflicts.

@mtrofin mtrofin marked this pull request as ready for review August 22, 2024 20:57
@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:ir llvm:transforms labels Aug 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2024 •
edited
Loading

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.

Full diff: https://github.com/llvm/llvm-project/pull/105731.diff

5 Files Affected:

  • (modified) llvm/include/llvm/IR/IntrinsicInst.h (+11-3)
  • (modified) llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp (+2-1)
  • (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+10-9)
  • (modified) llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll (+14-27)
  • (modified) llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll (+4-7)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 2f1e2c08c3ecec..65c4361767be93 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -1495,11 +1495,19 @@ class InstrProfInstBase : public IntrinsicInst {
       return isCounterBase(*Instr) || isMCDCBitmapBase(*Instr);
     return false;
   }
-  // The name of the instrumented function.
+
+  // The name of the instrumented function, assuming it is a global variable.
   GlobalVariable *getName() const {
-    return cast<GlobalVariable>(
-        const_cast<Value *>(getArgOperand(0))->stripPointerCasts());
+    return cast<GlobalVariable>(getNameValue());
+  }
+
+  // The "name" operand of the profile instrumentation instruction - this is the
+  // operand that can be used to relate the instruction to the function it
+  // belonged to at instrumentation time.
+  Value *getNameValue() const {
+    return const_cast<Value *>(getArgOperand(0))->stripPointerCasts();
   }
+
   // The hash of the CFG for the instrumented function.
   ConstantInt *getHash() const {
     return cast<ConstantInt>(const_cast<Value *>(getArgOperand(1)));
diff --git a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
index 9b10cbba84075a..43bebc99316e06 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
@@ -226,7 +226,8 @@ bool CtxInstrumentationLowerer::lowerFunction(Function &F) {
 
       IRBuilder<> Builder(Mark);
 
-      Guid = Builder.getInt64(AssignGUIDPass::getGUID(F));
+      Guid = Builder.getInt64(
+          AssignGUIDPass::getGUID(cast<Function>(*Mark->getNameValue())));
       // The type of the context of this function is now knowable since we have
       // NrCallsites and NrCounters. We delcare it here because it's more
       // convenient - we have the Builder.
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index b3644031c5a44b..450f69556ddd22 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -464,7 +464,7 @@ struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
   VisitMode Mode = VM_counting;  // Visiting mode.
   unsigned *CurCtrIdx = nullptr; // Pointer to current counter index.
   unsigned TotalNumCtrs = 0;     // Total number of counters
-  GlobalVariable *FuncNameVar = nullptr;
+  GlobalValue *FuncNameVar = nullptr;
   uint64_t FuncHash = 0;
   PGOUseFunc *UseFunc = nullptr;
   bool HasSingleByteCoverage;
@@ -482,7 +482,7 @@ struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
   // Ind is a pointer to the counter index variable; \p TotalNC
   // is the total number of counters; \p FNV is the pointer to the
   // PGO function name var; \p FHash is the function hash.
-  void instrumentSelects(unsigned *Ind, unsigned TotalNC, GlobalVariable *FNV,
+  void instrumentSelects(unsigned *Ind, unsigned TotalNC, GlobalValue *FNV,
                          uint64_t FHash) {
     Mode = VM_instrument;
     CurCtrIdx = Ind;
@@ -901,13 +901,14 @@ void FunctionInstrumenter::instrument() {
     SplitIndirectBrCriticalEdges(F, /*IgnoreBlocksWithoutPHI=*/false, BPI, BFI);
   }
 
+  const bool IsCtxProf = InstrumentationType == PGOInstrumentationType::CTXPROF;
   FuncPGOInstrumentation<PGOEdge, PGOBBInfo> FuncInfo(
-      F, TLI, ComdatMembers, true, BPI, BFI,
+      F, TLI, ComdatMembers, /*CreateGlobalVar=*/!IsCtxProf, BPI, BFI,
       InstrumentationType == PGOInstrumentationType::CSFDO,
       shouldInstrumentEntryBB(), PGOBlockCoverage);
 
-  auto Name = FuncInfo.FuncNameVar;
-  auto CFGHash =
+  auto *const Name = IsCtxProf ? cast<GlobalValue>(&F) : FuncInfo.FuncNameVar;
+  auto *const CFGHash =
       ConstantInt::get(Type::getInt64Ty(M.getContext()), FuncInfo.FunctionHash);
   if (PGOFunctionEntryCoverage) {
     auto &EntryBB = F.getEntryBlock();
@@ -925,7 +926,7 @@ void FunctionInstrumenter::instrument() {
   unsigned NumCounters =
       InstrumentBBs.size() + FuncInfo.SIVisitor.getNumOfSelectInsts();
 
-  if (InstrumentationType == PGOInstrumentationType::CTXPROF) {
+  if (IsCtxProf) {
     auto *CSIntrinsic =
         Intrinsic::getDeclaration(&M, Intrinsic::instrprof_callsite);
     // We want to count the instrumentable callsites, then instrument them. This
@@ -989,7 +990,7 @@ void FunctionInstrumenter::instrument() {
   }
 
   // Now instrument select instructions:
-  FuncInfo.SIVisitor.instrumentSelects(&I, NumCounters, FuncInfo.FuncNameVar,
+  FuncInfo.SIVisitor.instrumentSelects(&I, NumCounters, Name,
                                        FuncInfo.FunctionHash);
   assert(I == NumCounters);
 
@@ -1032,8 +1033,8 @@ void FunctionInstrumenter::instrument() {
       populateEHOperandBundle(Cand, BlockColors, OpBundles);
       Builder.CreateCall(
           Intrinsic::getDeclaration(&M, Intrinsic::instrprof_value_profile),
-          {FuncInfo.FuncNameVar, Builder.getInt64(FuncInfo.FunctionHash),
-           ToProfile, Builder.getInt32(Kind), Builder.getInt32(SiteIndex++)},
+          {Name, Builder.getInt64(FuncInfo.FunctionHash), ToProfile,
+           Builder.getInt32(Kind), Builder.getInt32(SiteIndex++)},
           OpBundles);
     }
   } // IPVK_First <= Kind <= IPVK_Last
diff --git a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
index df4e467567c46e..c94c2b4da57a98 100644
--- a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
+++ b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
@@ -9,19 +9,6 @@
 declare void @bar()
 
 ;.
-; INSTRUMENT: @__profn_foo = private constant [3 x i8] c"foo"
-; INSTRUMENT: @__profn_an_entrypoint = private constant [13 x i8] c"an_entrypoint"
-; INSTRUMENT: @__profn_another_entrypoint_no_callees = private constant [29 x i8] c"another_entrypoint_no_callees"
-; INSTRUMENT: @__profn_simple = private constant [6 x i8] c"simple"
-; INSTRUMENT: @__profn_no_callsites = private constant [12 x i8] c"no_callsites"
-; INSTRUMENT: @__profn_no_counters = private constant [11 x i8] c"no_counters"
-;.
-; LOWERING: @__profn_foo = private constant [3 x i8] c"foo"
-; LOWERING: @__profn_an_entrypoint = private constant [13 x i8] c"an_entrypoint"
-; LOWERING: @__profn_another_entrypoint_no_callees = private constant [29 x i8] c"another_entrypoint_no_callees"
-; LOWERING: @__profn_simple = private constant [6 x i8] c"simple"
-; LOWERING: @__profn_no_callsites = private constant [12 x i8] c"no_callsites"
-; LOWERING: @__profn_no_counters = private constant [11 x i8] c"no_counters"
 ; LOWERING: @an_entrypoint_ctx_root = global { ptr, ptr, ptr, i8 } zeroinitializer
 ; LOWERING: @another_entrypoint_no_callees_ctx_root = global { ptr, ptr, ptr, i8 } zeroinitializer
 ; LOWERING: @__llvm_ctx_profile_callsite = external hidden thread_local global ptr
@@ -30,16 +17,16 @@ declare void @bar()
 define void @foo(i32 %a, ptr %fct) {
 ; INSTRUMENT-LABEL: define void @foo(
 ; INSTRUMENT-SAME: i32 [[A:%.*]], ptr [[FCT:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[T]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1)
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
 ; INSTRUMENT-NEXT:    call void [[FCT]](i32 [[A]])
 ; INSTRUMENT-NEXT:    br label [[EXIT:%.*]]
 ; INSTRUMENT:       no:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
 ; INSTRUMENT-NEXT:    call void @bar()
 ; INSTRUMENT-NEXT:    br label [[EXIT]]
 ; INSTRUMENT:       exit:
@@ -92,12 +79,12 @@ exit:
 define void @an_entrypoint(i32 %a) {
 ; INSTRUMENT-LABEL: define void @an_entrypoint(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_an_entrypoint, i64 784007058953177093, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @an_entrypoint, i64 784007058953177093, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[T]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_an_entrypoint, i64 784007058953177093, i32 2, i32 1)
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_an_entrypoint, i64 784007058953177093, i32 1, i32 0, ptr @foo)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @an_entrypoint, i64 784007058953177093, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @an_entrypoint, i64 784007058953177093, i32 1, i32 0, ptr @foo)
 ; INSTRUMENT-NEXT:    call void @foo(i32 1, ptr null)
 ; INSTRUMENT-NEXT:    ret void
 ; INSTRUMENT:       no:
@@ -144,11 +131,11 @@ no:
 define void @another_entrypoint_no_callees(i32 %a) {
 ; INSTRUMENT-LABEL: define void @another_entrypoint_no_callees(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[T]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 1)
 ; INSTRUMENT-NEXT:    ret void
 ; INSTRUMENT:       no:
 ; INSTRUMENT-NEXT:    ret void
@@ -184,7 +171,7 @@ no:
 define void @simple(i32 %a) {
 ; INSTRUMENT-LABEL: define void @simple(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_simple, i64 742261418966908927, i32 1, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @simple, i64 742261418966908927, i32 1, i32 0)
 ; INSTRUMENT-NEXT:    ret void
 ;
 ; LOWERING-LABEL: define void @simple(
@@ -202,11 +189,11 @@ define void @simple(i32 %a) {
 define i32 @no_callsites(i32 %a) {
 ; INSTRUMENT-LABEL: define i32 @no_callsites(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_no_callsites, i64 784007058953177093, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @no_callsites, i64 784007058953177093, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[C:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[C]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_no_callsites, i64 784007058953177093, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @no_callsites, i64 784007058953177093, i32 2, i32 1)
 ; INSTRUMENT-NEXT:    ret i32 1
 ; INSTRUMENT:       no:
 ; INSTRUMENT-NEXT:    ret i32 0
@@ -238,8 +225,8 @@ no:
 
 define void @no_counters() {
 ; INSTRUMENT-LABEL: define void @no_counters() {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_no_counters, i64 742261418966908927, i32 1, i32 0)
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_no_counters, i64 742261418966908927, i32 1, i32 0, ptr @bar)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @no_counters, i64 742261418966908927, i32 1, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @no_counters, i64 742261418966908927, i32 1, i32 0, ptr @bar)
 ; INSTRUMENT-NEXT:    call void @bar()
 ; INSTRUMENT-NEXT:    ret void
 ;
diff --git a/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
index cb8ab78dc0f414..7959e4d0760edb 100644
--- a/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
+++ b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
@@ -7,22 +7,19 @@
 
 declare void @bar()
 
-;.
-; CHECK: @__profn_foo = private constant [3 x i8] c"foo"
-;.
 define void @foo(i32 %a, ptr %fct) {
 ; CHECK-LABEL: define void @foo(
 ; CHECK-SAME: i32 [[A:%.*]], ptr [[FCT:%.*]]) local_unnamed_addr !guid [[META0:![0-9]+]] {
-; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0)
+; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 0)
 ; CHECK-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; CHECK-NEXT:    br i1 [[T]], label %[[YES:.*]], label %[[NO:.*]]
 ; CHECK:       [[YES]]:
-; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1)
-; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
+; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 1)
+; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
 ; CHECK-NEXT:    call void [[FCT]](i32 0)
 ; CHECK-NEXT:    br label %[[EXIT:.*]]
 ; CHECK:       [[NO]]:
-; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
+; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label %[[EXIT]]
 ; CHECK:       [[EXIT]]:

@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2024

@llvm/pr-subscribers-pgo

Author: Mircea Trofin (mtrofin)

Changes

We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.

Full diff: https://github.com/llvm/llvm-project/pull/105731.diff

5 Files Affected:

  • (modified) llvm/include/llvm/IR/IntrinsicInst.h (+11-3)
  • (modified) llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp (+2-1)
  • (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+10-9)
  • (modified) llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll (+14-27)
  • (modified) llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll (+4-7)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 2f1e2c08c3ecec..65c4361767be93 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -1495,11 +1495,19 @@ class InstrProfInstBase : public IntrinsicInst {
       return isCounterBase(*Instr) || isMCDCBitmapBase(*Instr);
     return false;
   }
-  // The name of the instrumented function.
+
+  // The name of the instrumented function, assuming it is a global variable.
   GlobalVariable *getName() const {
-    return cast<GlobalVariable>(
-        const_cast<Value *>(getArgOperand(0))->stripPointerCasts());
+    return cast<GlobalVariable>(getNameValue());
+  }
+
+  // The "name" operand of the profile instrumentation instruction - this is the
+  // operand that can be used to relate the instruction to the function it
+  // belonged to at instrumentation time.
+  Value *getNameValue() const {
+    return const_cast<Value *>(getArgOperand(0))->stripPointerCasts();
   }
+
   // The hash of the CFG for the instrumented function.
   ConstantInt *getHash() const {
     return cast<ConstantInt>(const_cast<Value *>(getArgOperand(1)));
diff --git a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
index 9b10cbba84075a..43bebc99316e06 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
@@ -226,7 +226,8 @@ bool CtxInstrumentationLowerer::lowerFunction(Function &F) {
 
       IRBuilder<> Builder(Mark);
 
-      Guid = Builder.getInt64(AssignGUIDPass::getGUID(F));
+      Guid = Builder.getInt64(
+          AssignGUIDPass::getGUID(cast<Function>(*Mark->getNameValue())));
       // The type of the context of this function is now knowable since we have
       // NrCallsites and NrCounters. We delcare it here because it's more
       // convenient - we have the Builder.
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index b3644031c5a44b..450f69556ddd22 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -464,7 +464,7 @@ struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
   VisitMode Mode = VM_counting;  // Visiting mode.
   unsigned *CurCtrIdx = nullptr; // Pointer to current counter index.
   unsigned TotalNumCtrs = 0;     // Total number of counters
-  GlobalVariable *FuncNameVar = nullptr;
+  GlobalValue *FuncNameVar = nullptr;
   uint64_t FuncHash = 0;
   PGOUseFunc *UseFunc = nullptr;
   bool HasSingleByteCoverage;
@@ -482,7 +482,7 @@ struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
   // Ind is a pointer to the counter index variable; \p TotalNC
   // is the total number of counters; \p FNV is the pointer to the
   // PGO function name var; \p FHash is the function hash.
-  void instrumentSelects(unsigned *Ind, unsigned TotalNC, GlobalVariable *FNV,
+  void instrumentSelects(unsigned *Ind, unsigned TotalNC, GlobalValue *FNV,
                          uint64_t FHash) {
     Mode = VM_instrument;
     CurCtrIdx = Ind;
@@ -901,13 +901,14 @@ void FunctionInstrumenter::instrument() {
     SplitIndirectBrCriticalEdges(F, /*IgnoreBlocksWithoutPHI=*/false, BPI, BFI);
   }
 
+  const bool IsCtxProf = InstrumentationType == PGOInstrumentationType::CTXPROF;
   FuncPGOInstrumentation<PGOEdge, PGOBBInfo> FuncInfo(
-      F, TLI, ComdatMembers, true, BPI, BFI,
+      F, TLI, ComdatMembers, /*CreateGlobalVar=*/!IsCtxProf, BPI, BFI,
       InstrumentationType == PGOInstrumentationType::CSFDO,
       shouldInstrumentEntryBB(), PGOBlockCoverage);
 
-  auto Name = FuncInfo.FuncNameVar;
-  auto CFGHash =
+  auto *const Name = IsCtxProf ? cast<GlobalValue>(&F) : FuncInfo.FuncNameVar;
+  auto *const CFGHash =
       ConstantInt::get(Type::getInt64Ty(M.getContext()), FuncInfo.FunctionHash);
   if (PGOFunctionEntryCoverage) {
     auto &EntryBB = F.getEntryBlock();
@@ -925,7 +926,7 @@ void FunctionInstrumenter::instrument() {
   unsigned NumCounters =
       InstrumentBBs.size() + FuncInfo.SIVisitor.getNumOfSelectInsts();
 
-  if (InstrumentationType == PGOInstrumentationType::CTXPROF) {
+  if (IsCtxProf) {
     auto *CSIntrinsic =
         Intrinsic::getDeclaration(&M, Intrinsic::instrprof_callsite);
     // We want to count the instrumentable callsites, then instrument them. This
@@ -989,7 +990,7 @@ void FunctionInstrumenter::instrument() {
   }
 
   // Now instrument select instructions:
-  FuncInfo.SIVisitor.instrumentSelects(&I, NumCounters, FuncInfo.FuncNameVar,
+  FuncInfo.SIVisitor.instrumentSelects(&I, NumCounters, Name,
                                        FuncInfo.FunctionHash);
   assert(I == NumCounters);
 
@@ -1032,8 +1033,8 @@ void FunctionInstrumenter::instrument() {
       populateEHOperandBundle(Cand, BlockColors, OpBundles);
       Builder.CreateCall(
           Intrinsic::getDeclaration(&M, Intrinsic::instrprof_value_profile),
-          {FuncInfo.FuncNameVar, Builder.getInt64(FuncInfo.FunctionHash),
-           ToProfile, Builder.getInt32(Kind), Builder.getInt32(SiteIndex++)},
+          {Name, Builder.getInt64(FuncInfo.FunctionHash), ToProfile,
+           Builder.getInt32(Kind), Builder.getInt32(SiteIndex++)},
           OpBundles);
     }
   } // IPVK_First <= Kind <= IPVK_Last
diff --git a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
index df4e467567c46e..c94c2b4da57a98 100644
--- a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
+++ b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
@@ -9,19 +9,6 @@
 declare void @bar()
 
 ;.
-; INSTRUMENT: @__profn_foo = private constant [3 x i8] c"foo"
-; INSTRUMENT: @__profn_an_entrypoint = private constant [13 x i8] c"an_entrypoint"
-; INSTRUMENT: @__profn_another_entrypoint_no_callees = private constant [29 x i8] c"another_entrypoint_no_callees"
-; INSTRUMENT: @__profn_simple = private constant [6 x i8] c"simple"
-; INSTRUMENT: @__profn_no_callsites = private constant [12 x i8] c"no_callsites"
-; INSTRUMENT: @__profn_no_counters = private constant [11 x i8] c"no_counters"
-;.
-; LOWERING: @__profn_foo = private constant [3 x i8] c"foo"
-; LOWERING: @__profn_an_entrypoint = private constant [13 x i8] c"an_entrypoint"
-; LOWERING: @__profn_another_entrypoint_no_callees = private constant [29 x i8] c"another_entrypoint_no_callees"
-; LOWERING: @__profn_simple = private constant [6 x i8] c"simple"
-; LOWERING: @__profn_no_callsites = private constant [12 x i8] c"no_callsites"
-; LOWERING: @__profn_no_counters = private constant [11 x i8] c"no_counters"
 ; LOWERING: @an_entrypoint_ctx_root = global { ptr, ptr, ptr, i8 } zeroinitializer
 ; LOWERING: @another_entrypoint_no_callees_ctx_root = global { ptr, ptr, ptr, i8 } zeroinitializer
 ; LOWERING: @__llvm_ctx_profile_callsite = external hidden thread_local global ptr
@@ -30,16 +17,16 @@ declare void @bar()
 define void @foo(i32 %a, ptr %fct) {
 ; INSTRUMENT-LABEL: define void @foo(
 ; INSTRUMENT-SAME: i32 [[A:%.*]], ptr [[FCT:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[T]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1)
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
 ; INSTRUMENT-NEXT:    call void [[FCT]](i32 [[A]])
 ; INSTRUMENT-NEXT:    br label [[EXIT:%.*]]
 ; INSTRUMENT:       no:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
 ; INSTRUMENT-NEXT:    call void @bar()
 ; INSTRUMENT-NEXT:    br label [[EXIT]]
 ; INSTRUMENT:       exit:
@@ -92,12 +79,12 @@ exit:
 define void @an_entrypoint(i32 %a) {
 ; INSTRUMENT-LABEL: define void @an_entrypoint(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_an_entrypoint, i64 784007058953177093, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @an_entrypoint, i64 784007058953177093, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[T]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_an_entrypoint, i64 784007058953177093, i32 2, i32 1)
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_an_entrypoint, i64 784007058953177093, i32 1, i32 0, ptr @foo)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @an_entrypoint, i64 784007058953177093, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @an_entrypoint, i64 784007058953177093, i32 1, i32 0, ptr @foo)
 ; INSTRUMENT-NEXT:    call void @foo(i32 1, ptr null)
 ; INSTRUMENT-NEXT:    ret void
 ; INSTRUMENT:       no:
@@ -144,11 +131,11 @@ no:
 define void @another_entrypoint_no_callees(i32 %a) {
 ; INSTRUMENT-LABEL: define void @another_entrypoint_no_callees(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[T]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @another_entrypoint_no_callees, i64 784007058953177093, i32 2, i32 1)
 ; INSTRUMENT-NEXT:    ret void
 ; INSTRUMENT:       no:
 ; INSTRUMENT-NEXT:    ret void
@@ -184,7 +171,7 @@ no:
 define void @simple(i32 %a) {
 ; INSTRUMENT-LABEL: define void @simple(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_simple, i64 742261418966908927, i32 1, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @simple, i64 742261418966908927, i32 1, i32 0)
 ; INSTRUMENT-NEXT:    ret void
 ;
 ; LOWERING-LABEL: define void @simple(
@@ -202,11 +189,11 @@ define void @simple(i32 %a) {
 define i32 @no_callsites(i32 %a) {
 ; INSTRUMENT-LABEL: define i32 @no_callsites(
 ; INSTRUMENT-SAME: i32 [[A:%.*]]) {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_no_callsites, i64 784007058953177093, i32 2, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @no_callsites, i64 784007058953177093, i32 2, i32 0)
 ; INSTRUMENT-NEXT:    [[C:%.*]] = icmp eq i32 [[A]], 0
 ; INSTRUMENT-NEXT:    br i1 [[C]], label [[YES:%.*]], label [[NO:%.*]]
 ; INSTRUMENT:       yes:
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_no_callsites, i64 784007058953177093, i32 2, i32 1)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @no_callsites, i64 784007058953177093, i32 2, i32 1)
 ; INSTRUMENT-NEXT:    ret i32 1
 ; INSTRUMENT:       no:
 ; INSTRUMENT-NEXT:    ret i32 0
@@ -238,8 +225,8 @@ no:
 
 define void @no_counters() {
 ; INSTRUMENT-LABEL: define void @no_counters() {
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_no_counters, i64 742261418966908927, i32 1, i32 0)
-; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_no_counters, i64 742261418966908927, i32 1, i32 0, ptr @bar)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.increment(ptr @no_counters, i64 742261418966908927, i32 1, i32 0)
+; INSTRUMENT-NEXT:    call void @llvm.instrprof.callsite(ptr @no_counters, i64 742261418966908927, i32 1, i32 0, ptr @bar)
 ; INSTRUMENT-NEXT:    call void @bar()
 ; INSTRUMENT-NEXT:    ret void
 ;
diff --git a/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
index cb8ab78dc0f414..7959e4d0760edb 100644
--- a/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
+++ b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
@@ -7,22 +7,19 @@
 
 declare void @bar()
 
-;.
-; CHECK: @__profn_foo = private constant [3 x i8] c"foo"
-;.
 define void @foo(i32 %a, ptr %fct) {
 ; CHECK-LABEL: define void @foo(
 ; CHECK-SAME: i32 [[A:%.*]], ptr [[FCT:%.*]]) local_unnamed_addr !guid [[META0:![0-9]+]] {
-; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0)
+; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 0)
 ; CHECK-NEXT:    [[T:%.*]] = icmp eq i32 [[A]], 0
 ; CHECK-NEXT:    br i1 [[T]], label %[[YES:.*]], label %[[NO:.*]]
 ; CHECK:       [[YES]]:
-; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1)
-; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
+; CHECK-NEXT:    call void @llvm.instrprof.increment(ptr @foo, i64 728453322856651412, i32 2, i32 1)
+; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 0, ptr [[FCT]])
 ; CHECK-NEXT:    call void [[FCT]](i32 0)
 ; CHECK-NEXT:    br label %[[EXIT:.*]]
 ; CHECK:       [[NO]]:
-; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
+; CHECK-NEXT:    call void @llvm.instrprof.callsite(ptr @foo, i64 728453322856651412, i32 2, i32 1, ptr @bar)
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label %[[EXIT]]
 ; CHECK:       [[EXIT]]:

@mtrofin
[ctx_prof] Remove the dependency on the "name" GlobalVariable
f1780fa 
We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.
@mtrofin mtrofin force-pushed the users/mtrofin/08-22-_ctx_prof_remove_the_dependency_on_the_name_globalvariable branch from f9f3d08 to f1780fa Compare August 22, 2024 21:00
@mtrofin mtrofin mentioned this pull request Aug 23, 2024
Merged
snehasish
snehasish approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@snehasish snehasish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mtrofin Graphite App
Copy link
Member Author

mtrofin commented Aug 23, 2024 •
edited
Loading

Merge activity

  • Aug 23, 1:24 PM EDT: @mtrofin started a stack merge that includes this pull request via Graphite.
  • Aug 23, 1:25 PM EDT: Graphite couldn't merge this PR because it failed optional checks and "ignore optional checks" was not selected.
  • Aug 23, 1:29 PM EDT: @mtrofin started a stack merge that includes this pull request via Graphite.
  • Aug 23, 1:31 PM EDT: @mtrofin merged this pull request with Graphite.

@mtrofin mtrofin merged commit 960a210 into main Aug 23, 2024
6 of 8 checks passed
@mtrofin mtrofin deleted the users/mtrofin/08-22-_ctx_prof_remove_the_dependency_on_the_name_globalvariable branch August 23, 2024 17:31
5chmidti pushed a commit that referenced this pull request Aug 24, 2024
@mtrofin @5chmidti
[ctx_prof] Remove the dependency on the "name" GlobalVariable (#105731)
8d54b7d 
We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Aug 24, 2024
@ronlieb
merge main into amd-staging
094fb1d 
revert: ongoign Ctx
960a210 [ctx_prof] Remove the dependency on the "name" GlobalVariable (llvm#105731)

Change-Id: If937300a0217f863dda6dc71b62acc82e285d5fd
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this pull request Sep 2, 2024
@mtrofin @dmpolukhin
[ctx_prof] Remove the dependency on the "name" GlobalVariable (llvm#1…
a5ba3aa 
…05731)

We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Sep 7, 2024
@mtrofin @ronlieb
[ctx_prof] Remove the dependency on the "name" GlobalVariable (llvm#1…
b38773b 
…05731)

We don't need that name variable for contextual instrumentation, we just
use the function to get its GUID which we pass to the runtime, and rely
on metadata to capture it through the various optimization passes. This
change removes the need for the name global variable.

Change-Id: I06e95c9a9c74daa1529f8282227e868e55802a6d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snehasish snehasish snehasish approved these changes

@boomanaiden154 boomanaiden154 Awaiting requested review from boomanaiden154

@teresajohnson teresajohnson Awaiting requested review from teresajohnson

@ellishg ellishg Awaiting requested review from ellishg

@xur-llvm xur-llvm Awaiting requested review from xur-llvm

Assignees
No one assigned
Labels
llvm:ir llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mtrofin @boomanaiden154 @llvmbot @snehasish