[PATCH] D59431: [DebugInfo] Terminate more kinds of location-list ranges at the end of basic blocks

[Jeremy Morse via Phabricator via llvm-commits]

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


================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp:372
+          // to keep LiveEntries iteration stable.
+          if (DMI->getOperand(0).isReg()) {
+            unsigned RegNo = DMI->getOperand(0).getReg();
----------------
aprantl wrote:
> This comment is sort of misleading because your test also triggers for values where isIndirectDebugValue() is true. I think the code does what you want, but the condition also triggers for register-indirect values.
I've adjusted the comment to reflect reality; the termination of indirect DBG_VALUEs is the subject of some oddness (PR41600) that's preserved in this patch.


================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp:380
+            EntryIndex ClobIdx = DbgValues.startClobber(Pair.first, MBB.back());
+            DbgValues.getEntry(Pair.first, Idx).endEntry(ClobIdx);
+            IdxesToRemove.push_back(Idx);
----------------
dstenb wrote:
> dstenb wrote:
> > I think you can use `Ent` instead of `DbgValues.getEntry(Pair.first, Idx)` here?
> Oh, no. Sorry. As we may add an entry to the vector when creating the clobbering entry, `Ent` may be stale.
Alas, I found that out the hard way when writing it ._.


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

https://reviews.llvm.org/D59431