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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc marked 3 inline comments as done.
chandlerc added a comment.

Thanks all! Going to land this after a touch more testing so that my test runs start being faster. =D



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:521-527
+          (R.LowPC - BasePC) > (uint64_t)std::numeric_limits<uint32_t>::max()
+              ? std::numeric_limits<uint32_t>::max()
+              : (uint32_t)(R.LowPC - BasePC);
+      uint32_t RelativeHighPC =
+          (R.HighPC - BasePC) > (uint64_t)std::numeric_limits<uint32_t>::max()
+              ? std::numeric_limits<uint32_t>::max()
+              : (uint32_t)(R.HighPC - BasePC);
----------------
dblaikie wrote:
> JDevlieghere wrote:
> > 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.
> Worth pulling out a named constant for std::numeric_limits<uint32_t>::max() ? it's a long expression used 4 times in these long/wrapped lines.
Not sure that any name I come up with is better than `std::numeric_limits<uint32_t>::max()`, but sure. Combined with the suggestion above.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:571-573
+        const uint32_t IntervalStart = AddrMap[i].first;
+        const uint32_t IntervalEnd = AddrMap[i + 1].first;
+        const int IntervalDieIdx = AddrMap[i].second;
----------------
dblaikie wrote:
> top level const on locals is a bit uncommon (don't have to change it, just mentioning it in case there's some special motivation, or in case the choice is worth revisiting)
I added these to catch bugs where I was mutating something that wasn't a reference and I meant to be mutating something that was a reference. ::shrug::

I generally try to be pragmatic about this rather than dogmatic and make things const when useful to avoid mistakes.


https://reviews.llvm.org/D40987