[PATCH] D73725: [DebugInfo] Avoid a quadratic-complexity corner case in LiveDebugValues

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 2 inline comments as done.
jmorse added a comment.

Adrian wrote:

> We don't have an existing unit test for LexicalScopes that generates IR manually and tests a few common queries, do we?

Alas no, there doesn't appear to be any testing of LexicalScopes at all.



================
Comment at: llvm/lib/CodeGen/LexicalScopes.cpp:299
+  for (auto &R : InsnRanges) {
+    auto CurMBBIt = R.first->getParent()->getIterator();
+    auto EndBBIt = R.second->getParent()->getIterator();
----------------
aprantl wrote:
> Perhaps I'm missing something, but this looks like a really complicated way of writing a for(;;) loop?
It looks like I was missing caffeine at the time; I'll switch this to a for loop,


================
Comment at: llvm/lib/CodeGen/LexicalScopes.cpp:327
+  getMachineBasicBlocks(DL, Set);
+  return Set.count(MBB) != 0;
 }
----------------
vsk wrote:
> aprantl wrote:
> > `return Set.count(MBB);`
> > 
> > is idiomatic, I think.
> Does a LexicalScope always start at a MachineBasicBlock boundary? I.e. can `Set.count(MBB)` be non-zero even if no instruction in `MBB` has a location dominated by `Scope`?
LexicalScopes don't always start at block boundaries; but the ranges only ever cover those instructions that are definitely part of the scope, and are inclusive, so if one had:

  entry:
    // instructions in scope 1
    b %someblock
  unrelated:
    // instructions in scope 2
    b %another block
  someblock:
    // instructions in scope 1
    ret

And scope 2 was not a child of scope 1, then there would be two instruction ranges covering the "entry" and "someblock" block. The beginning and end of the ranges would point at instructions entirely in those two blocks, and "unrelated" wouldn't be reported as being in scope 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73725