[PATCH] D66599: [DebugInfo] Remove invalidated locations during LiveDebugValues

Thu Aug 22 10:34:27 PDT 2019

aprantl added inline comments.

================
Comment at: lib/CodeGen/LiveDebugValues.cpp:946
   VarLocSet &VLS = OutLocs[CurMBB];
-  Changed = VLS |= OpenRanges.getVarLocs();
+  Changed = VLS != OpenRanges.getVarLocs();
   // New OutLocs set may be different due to spill, restore or register
----------------
Makes sense.

================
Comment at: lib/CodeGen/LiveDebugValues.cpp:1306-1312
+
+  // We track what is on the pending worklist to avoid inserting the same
+  // thing twice.  We could avoid this with a custom priority queue, but this
+  // is probably not worth it.
+  SmallPtrSet<MachineBasicBlock *, 16> OnPending;
+
   while (!Worklist.empty() || !Pending.empty()) {
----------------
jmorse wrote:
> Not technically necessary change here but I think it's worthwhile: the "OnPending" set this function maintains clears itself by going out of scope and back in again. This misled me for a bit, thinking there was something magical going on that meant OnPending didn't need clearing. IMHO YMMV it's better to explicitly clear it (hunk below) for ease of understanding. (Or if it's too irrelevant, then never mind).
I'm not sure about this change. Now OnPending has state that needs to be reasoned about. Let's not roll this into this patch.

================
Comment at: test/DebugInfo/X86/live-debug-values-remove-range.ll:20
+
+define i32 @foo(i32 %bar) !dbg !4 {
+entry:
----------------
This is a really nice testcase!

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66599