[PATCH] D57584: [DebugInfo][DAG] Reduce SelectionDAGs reordering of variables referring to Arguments

[Jeremy Morse via Phabricator via llvm-commits]

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

In D57584#1380763 <https://reviews.llvm.org/D57584#1380763>, @bjope wrote:

> Nice! Me and @dstenb actually discussed this kind of problem earlier today. So quite fun (and surprising) to see a solution pop up in the inbox.


Glad to be of service :)

In D57584#1380829 <https://reviews.llvm.org/D57584#1380829>, @bjope wrote:

> Maybe I was a little bit too quick to say that this solved our problem. Our problem is related to SelectionDAGBuilder::EmitFuncArgumentDbgValue hoiting dbg.value instructions referring to arguments (even if we just use the value from an argument to describe another variable that is assigned later on in the function). We will file a bugzilla ticket for that.


It may be that replicating the IsParamOfFunc logic in this change into resolveDanglingDebugInfo resolves that. (I haven't explored doing that yet).



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5357
+    // need to let them dangle until they get an SDNode.
+    bool isParamOfFunc = isa<Argument>(V) && Variable->isParameter() &&
+                         Variable->getScope() == FuncInfo.Fn->getSubprogram();
----------------
aprantl wrote:
> bjope wrote:
> > Capitalize local variable name.
> Wouldn't this condition incorrectly also identify an inlined argument of a recursive function call as being an argument of the outermost non-inlined function? I think you may also need to check the inlinedAt of the dbg.value is null.
Ah yes, that would break. I've added a test that calls DISubprogram->describes here, lifted from r317702, would that be sufficient?


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

https://reviews.llvm.org/D57584