[PATCH] D146980: [Assignment Tracking] Coalesce dbg loc definitions with contiguous fragments

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added inline comments.


================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:585-589
+    // We've inserted the location into the map. The map will have coalesced
+    // adjacent intervals (variable fragments) that describe the same memory
+    // location. Use this knowledge to insert a debug location that describes
+    // that coalesced fragment. This may eclipse other locs we've just
+    // inserted. This is okay as redundant locs will be cleaned up later.
----------------
jmorse wrote:
> I wonder if there's a pathological case where we effectively store a history of defs that we then have to manually clear up, at reasonable expense.
> 
> Not really a criticism, everything in compilers has a pathological case, and this is clearly a net positive for compilation overall.
I think it would be possible to be smarter about when to insert certain defs or not, but this solution seemed to yield decent results with fairly trivial changes. It could be an area to look at in future rounds of performance improvements.


================
Comment at: llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp:684-688
       // Shorten `i` so that there's space to insert `f`.
       //      [ f ]
       // [  -   i   -  ]
       // +
       // [ i ][ f ][ i ]
----------------
jmorse wrote:
> In this scenario, would I be right in thinking that the code below shortens "i", inserts "f" in the middle, and then IntervalMap will coalesce them back together if they're the same? (Seems like the right behaviour).
Yes exactly that.


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

https://reviews.llvm.org/D146980