[PATCH] D112282: [llvm-profgen] Switch to DWARF-based symbol and ranges

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:584
+        if (R.second) {
+          ElfRangeInfo &Symbol = R.first->second;
+          Symbol.DWARFSymbolName = *It.first;
----------------
nit: Symbol -> RangeInfo


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:78
+// skip storing start offset here.
+struct ElfRangeInfo {
+  // DWARF-based symbol name.
----------------
We are using both elf ranges and dwarf ranges as the terms. Should we stick to one, say DwarfRangeInfo and DWARFRangesVectorTy or the other way around?


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:352
 
-  StringRef getFuncFromStartOffset(uint64_t Offset) {
-    auto I = FuncStartOffsetMap.find(Offset);
-    if (I == FuncStartOffsetMap.end())
-      return StringRef();
-    return I->second.first;
+  ElfRangeInfo *getElfRangeInfoForStartOffset(uint64_t Offset) {
+    auto I = StartOffset2ElfRangeInfoMap.find(Offset);
----------------
nit: getElfRangeInfoForStartOffset -> findElfRangeInfoForStartOffset to be consistent with the function naming below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112282/new/

https://reviews.llvm.org/D112282