[PATCH] D44610: [SelectionDAG] Support multiple dangling debug info for one value

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:120
+  /// DanglingDebugInfoVector - Helper type for DanglingDebugInfoMap.
+  typedef SmallVector<DanglingDebugInfo, 4> DanglingDebugInfoVector;
+
----------------
rnk wrote:
> Embedding SmallVector in DenseMap is usually an efficiency trap. DenseMap stores keys and values in the table, not in separate allocations, so the value of every unfilled hashtable entry is wasted. Even then, filled slots typically have one dbg.value.
> 
> TinyPtrVector was created for this use case, but it looks like the elements are not pointers so it will not work.
> 
> I guess I'd suggest using std::vector or `SmallVector<DangingDebugInfo, 0>` to minimize wasted inline storage.
Thanks a lot for the feedback. I was a little bit worried about this part. I haven't spent any time on optimizing it really, and this first draft was aiming at getting it working in some simple way.

Now when I got the feedback that this patch is on track regarding the functionality I will go on doing some tests regarding impact on compile speed/memory consumption. After all, these problems were seen when analysing https://bugs.llvm.org/show_bug.cgi?id=36417, so it would be a pity if we end up with bad compilation performance again when solving this problem.


Repository:
  rL LLVM

https://reviews.llvm.org/D44610