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.

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

[thinlto] Implement the import of vtable definitions. #72551

Closed
wants to merge 12 commits into from

Conversation

minglotus-6
Copy link
Contributor

In per-module summary generation, looks at instructions with vtable value profile metadata, and adds referenced vtables to the read-only refs of the function.

The related rfc is https://discourse.llvm.org/t/rfc-dynamic-type-profiling-and-optimizations-in-llvm/74600

…used to do virtual table address comparision for indirect-call-promotion.

The changes include:
1) Insert value profile intrinsics and lowering them.
   - Introduced INSTR_PROF_VTABLE_DATA to record per-vtable data.
   - Modified LLVM_PROF_RAW_HEADER to record the metadata for vtable profiles.
   - Test case in llvm/test/Transforms/PGOProfile/vtable_profile.ll
2) Tooling support in llvm-profdata to show the added vtable information
   - Changes are made in {raw,text,indexed} prof reader and/or writer to read/write vtable profile data.
   - Test cases added in llvm/test/tools/llvm-profdata
- In IndirectCallVisitor, instrument vtables and skip the rest (e.g., function pointers, etc).
- Add brief comments for helper functions, remove debugging logs and rename vtable name section to vtabnames.
…ntrol vtable

instrumentation, and record address range of vtables (in
VTableAddRangeToMD5Map).
- The new option is used in PGOInstrumentation.cpp and
  InstrProfiling.cpp to flag-control the instrumentation of static and
  runtime information. Static information includes vtable address range
  and name md5 hash, runtime information are types of a vtable value.
- Before this commit, VTableAddrToMD5Map records start and end of vtable
  address individually. After this commit, the map records a range to
  return MD5 iff the address is within the range. This is more accurate
  when runtime address is collected in one module but static vtable
  information is not recorded.
1. Format the code indentation in IndirectCallVisitor.h, and add a FIXME
   to do more efficient vtable instrumentation.
2. In InstrProfSymtab::finalizeSymtab, sort and uniquify
   VTableAddrRangeToMD5Map (somehow forgot this when preparing this
   patch, the full prototype includes profile-use did this already.)
- Set option '-enable-vtable-value-profiling' to false. This option should make this change no-op.
- For indexed profile format, store vtable names in one section by compressing them and storing blobs strings.
  - Before this commit, they are stored as OnDiskIterableHashTable but hash table is not necessary; vtable names are stored in indexed format profiles for llvm-profdata usage. Function profiles are on-disk hash table so compiler could look up profiles faster. Clean up lookuptrait and writer trait as a result.
  - In index profile reader, track {VTableNamePtr, CompressedVTableNameLen} as metadata so llvm-profdata could construct symtab by decompressing vtable names. Meanwhile, when compiler reads indexed profiles, no need to decompress vtables since symtab could be constructed from module IR.
- Use the PGOName of vtables when computing compressed strings and MD5 hash in instrumentation stage.
  - Previously, the vtable name is used. The PGOName and name of vtables are the same for global symbols. For local symbols, PGOName has the format 'filename:varname'
- In IndirectCallVisitor.h, use a heuristc to find address feeding instructions. Currently clang doesn't emit type intrinsics without '-fwhole-program-vtables'. '-fwhole-program-vtables' requires '-flto', and '-flto' might not be enabled at instrumentation phase.
- In ValueProfilePlugins.inc, skip landingpad and catchpad instructions (to keep them the first non-phi) when finding an insertion point for value profile intrinsic.
- In InstrProf.cpp, simplify string decompression code by adding helper functions.
- In llvm-profdata.cpp, print the number of instrumented vtable sites and vpstats.
- (Hopefully) fix indentation in llvm/test/tools/llvm-profdata/vtable-value-prof-basic.test
  - 'git diff' and 'vim' looks okay.
- Undo the change in
  compiler-rt/test/profile/Linux/instrprof-value-prof-warn.test This is
  not necessary since -enable-vtable-value-profiling is false by
  default.
…e main' on vtable branch; resolve merge conflict
1. Primarily, use vtable PGO name in compressed strings in helper
   function getVarName (around line 1115 in InstrProfiling.cpp)
2. Use version 12 in {InstrProfReader.cpp around line 1312, InstrProfWriter.cpp around line 607}.
   - Using version 11 is an overlook in 'git merge main' (a change in
     upstream main takes version 11 already).
3. In InstrProf.cpp and its header, rename 'readPGOFuncNameStrings' to a
   generic name, and make it a static function. pr/71566 is an NFC doing
   essentially the same thing.
4. A few code cleaning (e.g., delete debugging comment in
   raw-64-bits*.test)
- In per-module summary generation, looks at instructions with vtable
  value profile metadata, and adds referenced vtables to the read-only
  refs of the function.
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 0718c1a8405ac130d72cd3525befed2911618cc7 ad4ec98b508eae67133187a0948092da5fee8e36 -- compiler-rt/lib/profile/InstrProfiling.h compiler-rt/lib/profile/InstrProfilingBuffer.c compiler-rt/lib/profile/InstrProfilingInternal.h compiler-rt/lib/profile/InstrProfilingMerge.c compiler-rt/lib/profile/InstrProfilingPlatformLinux.c compiler-rt/lib/profile/InstrProfilingWriter.c llvm/include/llvm/Analysis/IndirectCallVisitor.h llvm/include/llvm/ProfileData/InstrProf.h llvm/include/llvm/ProfileData/InstrProfReader.h llvm/include/llvm/ProfileData/InstrProfWriter.h llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h llvm/lib/Analysis/ModuleSummaryAnalysis.cpp llvm/lib/Bitcode/Writer/BitcodeWriter.cpp llvm/lib/ProfileData/InstrProf.cpp llvm/lib/ProfileData/InstrProfReader.cpp llvm/lib/ProfileData/InstrProfWriter.cpp llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp llvm/tools/llvm-profdata/llvm-profdata.cpp
View the diff from clang-format here.
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index c9e295f0cf..85491d33bc 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -578,8 +578,9 @@ public:
     AddrToMD5Map.push_back(std::make_pair(Addr, MD5Val));
   }
 
-  /// Map the address range (i.e., [start_address, end_address]) of a variable to
-  /// its names' MD5 hash. This interface is only used by the raw profile header.
+  /// Map the address range (i.e., [start_address, end_address]) of a variable
+  /// to its names' MD5 hash. This interface is only used by the raw profile
+  /// header.
   void mapVTableAddress(uint64_t StartAddr, uint64_t EndAddr, uint64_t MD5Val) {
     VTableAddrRangeToMD5Map.push_back(
         std::make_pair(std::make_pair(StartAddr, EndAddr), MD5Val));
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index bb06670d34..613b2596e0 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1214,8 +1214,8 @@ void InstrProfiling::getOrCreateVTableProfData(GlobalVariable *GV) {
   UsedVars.push_back(Data);
 }
 
-GlobalVariable *
-InstrProfiling::setupProfileSection(InstrProfInstBase *Inc, InstrProfSectKind IPSK) {
+GlobalVariable *InstrProfiling::setupProfileSection(InstrProfInstBase *Inc,
+                                                    InstrProfSectKind IPSK) {
   GlobalVariable *NamePtr = Inc->getName();
 
   // Match the linkage and visibility of the name global.
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index e4bbb36de4..cd512fb730 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -1225,13 +1225,13 @@ static int merge_main(int argc, const char *argv[]) {
                  clEnumVal(sample, "Sample profile")));
   cl::opt<ProfileFormat> OutputFormat(
       cl::desc("Format of output profile"), cl::init(PF_Ext_Binary),
-      cl::values(
-          clEnumValN(PF_Binary, "binary", "Binary encoding"),
-          clEnumValN(PF_Ext_Binary, "extbinary", "Extensible binary encoding "
-                     "(default)"),
-          clEnumValN(PF_Text, "text", "Text encoding"),
-          clEnumValN(PF_GCC, "gcc",
-                     "GCC encoding (only meaningful for -sample)")));
+      cl::values(clEnumValN(PF_Binary, "binary", "Binary encoding"),
+                 clEnumValN(PF_Ext_Binary, "extbinary",
+                            "Extensible binary encoding "
+                            "(default)"),
+                 clEnumValN(PF_Text, "text", "Text encoding"),
+                 clEnumValN(PF_GCC, "gcc",
+                            "GCC encoding (only meaningful for -sample)")));
   cl::opt<FailureMode> FailureMode(
       "failure-mode", cl::init(failIfAnyAreInvalid), cl::desc("Failure mode:"),
       cl::values(
@@ -1240,7 +1240,8 @@ static int merge_main(int argc, const char *argv[]) {
                      "Fail if any profile is invalid."),
           clEnumValN(failIfAllAreInvalid, "all",
                      "Fail only if all profiles are invalid.")));
-  cl::opt<bool> OutputSparse("sparse", cl::init(false),
+  cl::opt<bool> OutputSparse(
+      "sparse", cl::init(false),
       cl::desc("Generate a sparse profile (only meaningful for -instr)"));
   cl::opt<unsigned> NumThreads(
       "num-threads", cl::init(0),

@minglotus-6
Copy link
Contributor Author

Superseded by #79381

@minglotus-6 minglotus-6 closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant