[PATCH] D38068: [DebugInfo] Use a DenseMap to coalesce MachineOperand locations

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 2 inline comments as done.
rnk added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineOperand.h:785
 
+  // Artificial kinds for DenseMap usage.
+  enum : unsigned char {
----------------
aprantl wrote:
> `///`
Done, but I think it's private so it won't show up in Doxygen anyway.


================
Comment at: llvm/include/llvm/CodeGen/MachineOperand.h:787
+  enum : unsigned char {
+    MO_Empty = MO_Predicate + 1,
+    MO_Tombstone,
----------------
hans wrote:
> I wonder if this could be made future-proof in case another MO_ enumerator is ever added. Maybe add a MO_LAST = MO_Predicate to the enumeration?
Good idea, done.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:970
+
+    // Insert the new location and record its number.
+    auto InsertResult = NewLocations.insert({Loc, false});
----------------
hans wrote:
> Should this comment on the fact that coalescing can happen here if the new location is identical to something already in NewLocations?
The coalescing really happens below when we update the IntervalMap. I'll elaborate there on what it is.


https://reviews.llvm.org/D38068