[PATCH] D74706: [WIP][DebugInfo][LiveDebugValues] Index variable location IDs by machine location

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

Nice. I hacked up a similar version of this patch that defined VarLocMap as `DenseMap<unsigned, UniqueVector<VarLoc>>`. Similar to here, the idea was to maintain a UniqueVector<VarLoc> for each register. The trick here was to make "ID"s 64-bit, so that they could efficiently encode a (register, index into the UniqueVector for that register) pair. To do a lookup given an ID, we'd break apart the 64-bit ID to grab the UniqueVector for the register (non-register locations were all slapped into the '0'th UniqueVector), then index into the vector. This seems less general than your patch.

When I was putting the finishing touches on this, I took a closer look at my profiling run with D74633 <https://reviews.llvm.org/D74633> applied and saw this:

  2.49 min   27.8%	0 s	LiveDebugValues::process
  2.41 min   27.0%	5.40 s	LiveDebugValues::transferRegisterDef
  1.51 min   16.9%	1.51 min LiveDebugValues::VarLoc::isDescribedByReg() const
  32.73 s    6.1%		8.70 s	 llvm::SparseBitVector<128u>::SparseBitVectorIterator::operator++()

It looks like it costs us 33 seconds to iterate over OpenLocs /once/. If we were to do this once per location, I fear performance would regress. Well, at least for this particular WebKit test case I'm looking at, which has truly massive basic blocks.

So, while I think there is potential here, I'm not sure it's architecturally a good fit for `transferRegisterDef` (although, I readily admit it could be a good fit elsewhere).

What I was thinking of as an alternative (haven't had time to hack this up, might get to it today if my bug queue permits) is to replace our use of SparseBitVector with an IntervalSet. For prototyping purposes I was thinking of implementing this as an llvm::IntervalMap, equipped with set ops like `intersectWithComplement`, and a fancy iterator that lets us write `for (unsigned ID : theIntervalSet)`. My intuition/expectation is that the pointer chasing in SparseBitVector is causing all kinds of issues: if that's correct, and we fix that first, that makes space for investigating cool alternative layouts for VarLocMap.

wdyt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74706