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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 6 inline comments as done.
jmorse added inline comments.


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:139
+    using DbgValueLoc = std::pair<SlotIndex, MachineInstr*>;
+    std::map<unsigned, std::set<DbgValueLoc>> DbgVRegToValues;
+
----------------
aprantl wrote:
> 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?
I was going for strong ordering guarantees -- but it turns out that it isn't really necessary to erase elements in the body of the pass, so a sorted vector works just fine. Thanks for the tip!


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:3474
+        DbgValuesSeen = true;
+      } else if (!MI.isDebugInstr() && DbgValuesSeen) {
+        CurrentSlot = Slots.getInstructionIndex(MI);
----------------
aprantl wrote:
> This flip-flopping of DbgValuesSeen is hard to follow .. is there something more obvious that could be done? Otherwise, can it be documented?
I realised I could just rely on there not being any DBG_VALUEs in the ToInsert vector instead of explicitly tracking these things, so I've just deleted that flag.


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

https://reviews.llvm.org/D64630