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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:6931
       // where said address is used.
       if (!DVI || (DVI->getValue() && isa<AllocaInst>(DVI->getValue()))) {
         PrevNonDbgInst = Insn;
----------------
I think that the special handling of alloca can be removed now (either the alloca dominates the DVI and everyhing is fine, or is actually is possible that the DVI dominates the alloca, then I think we can move it).
Although, no need to make such a change in this patch if you think it is risky/unrelated.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:6937
       Instruction *VI = dyn_cast_or_null<Instruction>(DVI->getValue());
+      if (VI && DT.dominates(VI, DVI))
+        continue;
----------------
aprantl wrote:
> Please add a comment that explains why
I think this explanation can be added by updating the description of this function. Mentioning that the purpose is to resolve use-before-def problems (which really should be seen as bugs in this, as well as other, passes):
  "This function only moves (sinks) DbgValueInst:s when there is a use-before-def situation."


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58453