From de48df822d22e65d3e8e9417d78b46414a8a1126 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Mon, 22 Jul 2024 11:57:36 -0700 Subject: [PATCH] [InstrPGO][TypeProf]Annotate vtable types when they are present in the profile (#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 --- .../Linux/instrprof-vtable-value-prof.cpp | 27 ++++++++++++++++--- .../Instrumentation/PGOInstrumentation.cpp | 21 +++++++++++++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp b/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp index 24a96a227393ad3..411640fc5176d07 100644 --- a/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp +++ b/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp @@ -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 @@ -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 diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index 4f0ccc8f962dbf7..1ce8f58c1aa1408 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -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); @@ -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 &CountFromProfile); @@ -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(