[PATCH] D64630: [DebugInfo] Address performance regression with r364515

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:139
+    using DbgValueLoc = std::pair<SlotIndex, MachineInstr*>;
+    std::map<unsigned, std::set<DbgValueLoc>> DbgVRegToValues;
+
----------------
This sets of my "this looks expensive" alarm.
Could this be more efficiently be replaced by a `DenseMap` or a sorted `std::vector` + `std::lower_bound()`, or is this the best choice here? Similarly, should the `std::set` be something more compact?


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:343
+    /// Walk over function and initialize the DbgVRegToValues map.
+    void buildDebugMap(MachineFunction &MF);
+
----------------
`buildVRegToDbgValueMap()` or something more descriptive?


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:3474
+        DbgValuesSeen = true;
+      } else if (!MI.isDebugInstr() && DbgValuesSeen) {
+        CurrentSlot = Slots.getInstructionIndex(MI);
----------------
This flip-flopping of DbgValuesSeen is hard to follow .. is there something more obvious that could be done? Otherwise, can it be documented?


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:3563
+  // DBG_VALUEs for Reg at the same time. Advance whichever one has the lowest
+  // slot index. This relies on the DbgValueSet being a std::set, and thus
+  // its iteration order.
----------------
Ah. That might be the answer.


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

https://reviews.llvm.org/D64630