[PATCH] D74986: [LiveDebugValues] Encode register location within VarLoc IDs [3/3]

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D74986#2058583 <https://reviews.llvm.org/D74986#2058583>, @nikic wrote:

> In D74986#2057820 <https://reviews.llvm.org/D74986#2057820>, @vsk wrote:
>
> > In D74986#2057531 <https://reviews.llvm.org/D74986#2057531>, @nikic wrote:
> >
> > > While bisecting compile-time regressions, I hit on this change as a larger -O0 -g regression. It's 2% geomean on CTMark, with the largest regression being 4% on sqlite3. Might be worth a look.
> >
> >
> > @nikic We actually saw a similar result on our CTMark bot, but the regression was fixed by D76465 <https://reviews.llvm.org/D76465> + D76467 <https://reviews.llvm.org/D76467>. Were you able to measure any improvement after those changes (or, put another way, does reverting D74986 <https://reviews.llvm.org/D74986> + D76465 <https://reviews.llvm.org/D76465> + D76467 <https://reviews.llvm.org/D76467> fix the issue)?
>
>
> Thanks for the references! When I revert these patches in reverse order, I see (for the instructions retired metric) the original regression for sqlite at +4.4%, the CoalescingBitVector optimization as a -1.1% improvement, and the collectIDsForRegs() optimization as a -0.5% improvement. So these two changes seem to recover a sizable part of the regression (about a 35%), but not the entirety. As such, there might still be some optimization potential here. (Results for reference <http://llvm-compile-time-tracker.com/?config=O0-g&stat=instructions&branch=nikic%2Fperf%2Flive-debug-values> -- these are reverts, so inverted.)


Interesting; I missed this because I only looked at averaged wall times while investigating. Thanks for sharing this data (the tracker is awesome). Would you mind adding reverts of D74985 <https://reviews.llvm.org/D74985> + D74633 <https://reviews.llvm.org/D74633> to nikic/perf/live-debug-values so we can get a complete picture? Those are two NFC preparatory changes for the main data structure change (this patch). I suspect D74633 <https://reviews.llvm.org/D74633> in particular gave a large speedup: it's possible that this patch causes a slowdown relative to D74633 <https://reviews.llvm.org/D74633> on small inputs. That could tell us which direction to go. There a number of compile-time optimizations that are possible with CoalescingBitVector, but if we're slower on small inputs after reverting D74633 <https://reviews.llvm.org/D74633>, maybe we should rethink the data structure instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74986