[PATCH] D37078: [DebugInfo] Handle memory locations in LiveDebugValues

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5173
+    // node map.
+    // FIXME: Handle byval/inalloca params as above.
+    if (const AllocaInst *AI = dyn_cast<AllocaInst>(V)) {
----------------
rnk wrote:
> rnk wrote:
> > aprantl wrote:
> > > I think this should be a separate commit?
> > Ugh, this doesn't actually do the right thing. Here's my test that comes here:
> > ```
> > void useptr(int *);
> > void use_alloca(int cond) {
> >   int x = 0;
> >   int *px = &x;
> >   if (cond)
> >     useptr(px);
> > }
> > ```
> > 
> > After optimization, `px` will be described by a dbg.value of x's alloca. This code in InstrEmitter.cpp incorrectly builds an "indirect in memory" DBG_VALUE for px:
> > ```
> >   if (SD->getKind() == SDDbgValue::FRAMEIX) {
> >     // Stack address; this needs to be lowered in target-dependent fashion.
> >     // EmitTargetCodeForFrameDebugValue is responsible for allocation.
> >     return BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE))
> >         .addFrameIndex(SD->getFrameIx())
> >         .addImm(0)
> >         .addMetadata(Var)
> >         .addMetadata(Expr);
> >   }
> > ```
> > 
> > I guess I'll need to fix that to look for DW_OP_deref.
> Alternatively, you could say I misunderstood `DAG.getFrameIndexDbgValue`.
I think it would be best to split the SelectionDAG change out into a separate review and have a MIR-only test for LiveDebugValues. The LiveDebugValues change should be generally good and uncontroversial (apart from performance considerations) and this change looks like it will generate a longer discussion.


https://reviews.llvm.org/D37078