[PATCH] D40987: Rewrite the cached map used for locating the most precise DIE among inlined subroutines for a given address.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

Thanks for working on this Chandler! It took me a while to fully grasp why you needed the parent interval in the second layer but it makes sense to me. I was considering a slightly different approach approach where you would do a depth first traversal and add the address ranges as you encounter them, but that would presumably just complicate getting the LowPC mapped to the most nested (most precise) inlined subroutine.



================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h:216
+    /// represents the relative PC for an interval in the map. The second int
+    /// represents the indexinto the `InlinedSubroutineDIEs` vector of the DIE
+    /// that interval maps to. An index of '-1` indicates an empty mapping. The
----------------
s/indexinto/index into/


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:378
+    int DIEIndex = SubprogramDIEAddrInfos.size();
+    SubprogramDIEAddrInfos.push_back({Die, (uint64_t)-1, {}});
     for (const auto &R : Die.getAddressRanges()) {
----------------
I'm curious if there is any particular reason you prefer `::push_back({})` over `::emplace_back`? 


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:521
+      uint32_t RelativeLowPC =
+          (R.LowPC - BasePC) > (uint64_t)std::numeric_limits<uint32_t>::max()
+              ? std::numeric_limits<uint32_t>::max()
----------------
Maybe extract `(uint64_t)std::numeric_limits<uint32_t>::max()` to make this a little more readable?

Not relevant here (as it actually improves readability) but I wonder what the consensus is on C-style casts in LLVM? I don't think the style guide actually mentions it.


https://reviews.llvm.org/D40987