Skip to content

Commit

Permalink
[InstrPGO][TypeProf]Annotate vtable types when they are present in th…
Browse files Browse the repository at this point in the history
…e profile (llvm#99402)

Before this change, when `file.profdata` have vtable profiles but `--enable-vtable-value-profiling` is not on for optimized build, warnings from this line [1] will show up. They are benign for performance but confusing.

It's better to automatically annotate vtable profiles if `file.profdata` has them. This PR implements it in profile use pass.
* If `-icp-max-num-vtables` is zero (default value is 6), vtable profiles won't be annotated.

[1] https://github.com/llvm/llvm-project/blob/464d321ee8dde1eaf14b5537eaf030e6df513849/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1762-L1768
  • Loading branch information
minglotus-6 authored and sgundapa committed Jul 23, 2024
1 parent 67f8f46 commit de48df8
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
27 changes: 23 additions & 4 deletions compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,25 @@
// ICTEXT: {{.*}}instrprof-vtable-value-prof.cpp;_ZTVN12_GLOBAL__N_18Derived2E:750
// ICTEXT: _ZTV8Derived1:250

// When vtable value profiles exist, pgo-instr-use pass should annotate them
// even if `-enable-vtable-value-profiling` is not explicitly on.
// RUN: %clangxx -m64 -fprofile-use=test.profdata -fuse-ld=lld -O2 \
// RUN: -mllvm -print-after=pgo-instr-use -mllvm -filter-print-funcs=main \
// RUN: -mllvm -print-module-scope %s 2>&1 | FileCheck %s --check-prefix=ANNOTATE

// ANNOTATE-NOT: Inconsistent number of value sites
// ANNOTATE: !{!"VP", i32 2

// When vtable value profiles exist, pgo-instr-use pass will not annotate them
// if `-icp-max-num-vtables` is set to zero.
// RUN: %clangxx -m64 -fprofile-use=test.profdata -fuse-ld=lld -O2 \
// RUN: -mllvm -icp-max-num-vtables=0 -mllvm -print-after=pgo-instr-use \
// RUN: -mllvm -filter-print-funcs=main -mllvm -print-module-scope %s 2>&1 | \
// RUN: FileCheck %s --check-prefix=OMIT

// OMIT: Inconsistent number of value sites
// OMIT-NOT: !{!"VP", i32 2

// Test indirect call promotion transformation using vtable profiles.
// - Build with `-g` to enable debug information.
// - In real world settings, ICP pass is disabled in prelink pipeline. In
Expand All @@ -128,12 +147,12 @@
// RUN: | FileCheck %s --check-prefixes=REMARK,IR --implicit-check-not="!VP"

// For the indirect call site `ptr->func`
// REMARK: instrprof-vtable-value-prof.cpp:205:19: Promote indirect call to _ZN12_GLOBAL__N_18Derived24funcEii with count 150 out of 200, sink 1 instruction(s) and compare 1 vtable(s): {_ZTVN12_GLOBAL__N_18Derived2E}
// REMARK: instrprof-vtable-value-prof.cpp:205:19: Promote indirect call to _ZN8Derived14funcEii with count 50 out of 50, sink 1 instruction(s) and compare 1 vtable(s): {_ZTV8Derived1}
// REMARK: instrprof-vtable-value-prof.cpp:226:19: Promote indirect call to _ZN12_GLOBAL__N_18Derived24funcEii with count 150 out of 200, sink 1 instruction(s) and compare 1 vtable(s): {_ZTVN12_GLOBAL__N_18Derived2E}
// REMARK: instrprof-vtable-value-prof.cpp:226:19: Promote indirect call to _ZN8Derived14funcEii with count 50 out of 50, sink 1 instruction(s) and compare 1 vtable(s): {_ZTV8Derived1}
//
// For the indirect call site `delete ptr`
// REMARK: instrprof-vtable-value-prof.cpp:207:5: Promote indirect call to _ZN12_GLOBAL__N_18Derived2D0Ev with count 750 out of 1000, sink 2 instruction(s) and compare 1 vtable(s): {_ZTVN12_GLOBAL__N_18Derived2E}
// REMARK: instrprof-vtable-value-prof.cpp:207:5: Promote indirect call to _ZN8Derived1D0Ev with count 250 out of 250, sink 2 instruction(s) and compare 1 vtable(s): {_ZTV8Derived1}
// REMARK: instrprof-vtable-value-prof.cpp:228:5: Promote indirect call to _ZN12_GLOBAL__N_18Derived2D0Ev with count 750 out of 1000, sink 2 instruction(s) and compare 1 vtable(s): {_ZTVN12_GLOBAL__N_18Derived2E}
// REMARK: instrprof-vtable-value-prof.cpp:228:5: Promote indirect call to _ZN8Derived1D0Ev with count 250 out of 250, sink 2 instruction(s) and compare 1 vtable(s): {_ZTV8Derived1}

// The IR matchers for indirect callsite `ptr->func`.
// IR-LABEL: @main
Expand Down
21 changes: 19 additions & 2 deletions llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ class PGOUseFunc {
: F(Func), M(Modu), BFI(BFIin), PSI(PSI),
FuncInfo(Func, TLI, ComdatMembers, false, BPI, BFIin, IsCS,
InstrumentFuncEntry, HasSingleByteCoverage),
FreqAttr(FFA_Normal), IsCS(IsCS) {}
FreqAttr(FFA_Normal), IsCS(IsCS), VPC(Func, TLI) {}

void handleInstrProfError(Error Err, uint64_t MismatchedFuncSum);

Expand Down Expand Up @@ -1178,6 +1178,8 @@ class PGOUseFunc {
// Is to use the context sensitive profile.
bool IsCS;

ValueProfileCollector VPC;

// Find the Instrumented BB and set the value. Return false on error.
bool setInstrumentedCounts(const std::vector<uint64_t> &CountFromProfile);

Expand Down Expand Up @@ -1755,8 +1757,23 @@ void PGOUseFunc::annotateValueSites() {
void PGOUseFunc::annotateValueSites(uint32_t Kind) {
assert(Kind <= IPVK_Last);
unsigned ValueSiteIndex = 0;
auto &ValueSites = FuncInfo.ValueSites[Kind];

unsigned NumValueSites = ProfileRecord.getNumValueSites(Kind);

// Since there isn't a reliable or fast way for profile reader to tell if a
// profile is generated with `-enable-vtable-value-profiling` on, we run the
// value profile collector over the function IR to find the instrumented sites
// iff function profile records shows the number of instrumented vtable sites
// is not zero. Function cfg already takes the number of instrumented
// indirect call sites into account so it doesn't hash the number of
// instrumented vtables; as a side effect it makes it easier to enable
// profiling and profile use in two steps if needed.
// TODO: Remove this if/when -enable-vtable-value-profiling is on by default.
if (NumValueSites > 0 && Kind == IPVK_VTableTarget &&
NumValueSites != FuncInfo.ValueSites[IPVK_VTableTarget].size() &&
MaxNumVTableAnnotations != 0)
FuncInfo.ValueSites[IPVK_VTableTarget] = VPC.get(IPVK_VTableTarget);
auto &ValueSites = FuncInfo.ValueSites[Kind];
if (NumValueSites != ValueSites.size()) {
auto &Ctx = M->getContext();
Ctx.diagnose(DiagnosticInfoPGOProfile(
Expand Down

0 comments on commit de48df8

Please sign in to comment.