[PATCH] D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics

[Jeremy Morse via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:7228
+      // to move the dbg.value.
+      if (VI && DT.dominates(VI, DVI))
+        continue;
----------------
aprantl wrote:
> Is this expensive?
My understanding of dominator queries is that they're inexpensive, becoming just a hash table lookup of the instruction pointer. Building the dominator tree is what's expensive (at the start of placeDbgValues). It looks like CodeGenPrepare does keep a cached copy, but repeatedly invalidates and recomputes it as it changes things in the Function.

IMO building the dominator tree at the start of the function shouldn't be "Too" (TM) expensive, and I have low confidence that we could re-use the one CodeGenPrepare keeps around: there might be a code change that invalidates it by the point placeDbgValues is called that we're not aware of.


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

https://reviews.llvm.org/D58453