[clang] [compiler-rt] [llvm] [InstrFDO][TypeProf] Implement binary instrumentation and profile read/write (PR #66825)

Mingming Liu via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 5 22:09:29 PST 2024


================
@@ -490,6 +591,23 @@ Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) {
   return Error::success();
 }
 
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+  finalizeSymtab();
+  auto It = lower_bound(
+      VTableAddrRangeToMD5Map, Address,
----------------
minglotus-6 wrote:

IntervalMap could save the {sort, unique, lower_bound} over vectors. I tried to use it, but I thinkits implementation overhead doesn't make it an obvious win over STL usages 
1) IntervalMap [coalesces](https://github.com/llvm/llvm-project/blob/7bad74e66756ca2fd1fe4f5864e7123fb4553d78/llvm/include/llvm/ADT/IntervalMap.h#L127) adjacent ranges. `[a, b)` and `[c, d)` are considered adjacent `b + 1 == c` with the [default](https://github.com/llvm/llvm-project/blob/7bad74e66756ca2fd1fe4f5864e7123fb4553d78/llvm/include/llvm/ADT/IntervalMap.h#L155) trait. While vtables in `.data.rel.ro` may not be adjacent in real binaries,  a custom trait which doesn't coalesce adjacent ranges is more foolproof and less likely to go wrong.
2) The code might look a little quirky w.r.t class method interfaces.
     a) This [line](https://github.com/llvm/llvm-project/blob/7bad74e66756ca2fd1fe4f5864e7123fb4553d78/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp#L901) uses the implicit move assignment operator of `InstrProfSymtab`.
     b) `IntervalMap` constructor [requires](https://github.com/llvm/llvm-project/blob/7bad74e66756ca2fd1fe4f5864e7123fb4553d78/llvm/include/llvm/ADT/IntervalMap.h#L1041) an allocator, and the allocator is owned outside of interval map.
     c) Allocator doesn't have implicit move assignment operator because it [defines](https://github.com/llvm/llvm-project/blob/7bad74e66756ca2fd1fe4f5864e7123fb4553d78/llvm/include/llvm/Support/RecyclingAllocator.h#L37) destructor. 
   Considering a) and c), if 'InstrProfSymtab' has 'allocator' as a member, it needs to implement move assignment operator. Alternatively, since only raw profile reader sees profiled address and uses this map, raw profile reader could have an allocator member and pass it to `InstrSymtab::create` for interval map creation. But this way the code doesn't look very clean either.


https://github.com/llvm/llvm-project/pull/66825


More information about the cfe-commits mailing list