[PATCH] LowerDbgDeclare - get rid of redundant dbg.values when an alloca survives optimization

[David Blaikie]

On Tue, Oct 29, 2013 at 11:41 AM, Adrian Prantl <aprantl at apple.com> wrote:

> Hi Eric and David,
>
> On Oct 21, 2013, at 17:07, Eric Christopher <echristo at gmail.com> wrote:
> Cool deal. I've got some ideas on how to refactor/rewrite some of our
> > loc handling anyhow, but it's going to be a few days before it's
> > ready.
>
>
> I had some time to look into this some more and I learned a couple of
> interesting things:
> - The ##DEBUG_VALUE: comments in the asm output are really just comments
> do not necessarily show up in the DWARF. This is maybe not surprising, but
> what was surprising to me are the circumstances under which a DBG_VALUE
> makes it into the location list of a local variable:
> - dbg.declare and dbg.value intrinsics are mutually exclusive; the
> presence of a dbg.declare will shadow any dbg.values that refer to the same
> variable. As soon as there is an entry in the MMI map
> (FunctionLowering::set() in FunctionLowering.cpp:134), the dbg.values will
> be ignored by DwarfDebug::collectVariableInfo (DwarfDebug.cpp::1366).
> - Simply disabling that check will result in two separate DIEs for the
> same variable.
>
> This means that while my patch was correct based on how DwarfDebug
> currently behaves, that behavior is broken and I should instead allow for
> dbg.values and dbg.declares to be coalesced into a single location list.
>
> Before I dive into this I’d be curious to hear more about your ideas for
> location handling that you mentioned in your last mail, so we don’t
> accidentally evolve the code into two different directions.
>
>
> I also found a second, related issue that I’d like to tackle.
>
> This example:
>
> > foo(int map)
> > {
> >   lookup(&map);
> >   if (!verify(map)) {  }
> > }
> >
>
> results in the following DWARF:
>
> > 0x00000027:     TAG_subprogram [2] *
> >                  AT_name( "foo” )
> >                  AT_low_pc( 0x0000000000000000 )
> >                  AT_high_pc( 0x0000000000000026 )
> >               ...
> > 0x00000046:         TAG_formal_parameter [3]
> >                      AT_name( "map” )
> >                      AT_type( {0x00000075} ( int ) )
> >                      AT_location( fbreg -4 )
> >
> > 0x00000054:         TAG_lexical_block [4] *
> >                      AT_low_pc( 0x0000000000000016 )
> >                      AT_high_pc( 0x0000000000000020 )
> >
> > 0x00000065:             TAG_formal_parameter [3]
> >                          AT_name( "map" )
> >                          AT_type( {0x00000075} ( int ) )
> >                          AT_location( fbreg -4 )
>
> I would like teach DWARFDebug::constructScopeDIE() to recognize if a
> variable DIE describes a “subset" of a DIE in one of its parent lexical
> blocks.
>

To be clear, this just looks buggy - we shouldn't have two formal
parameters here... there is only one formal parameter to 'foo'. If we
create two DIEs that's a bug, we shouldn't do that.


>
> -- adrian
>
> >
> >
> > On Mon, Oct 21, 2013 at 5:06 PM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >> Yes, I definitely need to dig deeper here. I’m really wondering why
> those dbg.values show up in the assembly, but not in the DWARF output.
> >>
> >> I’ll keep you updated once I understand this better.
> >>
> >> -- adrian
> >>
> >> On Oct 21, 2013, at 16:54, Eric Christopher <echristo at gmail.com> wrote:
> >>
> >>> If the sections of text are contiguous we should have a single
> >>> location entry for the variable, else we should have multiple in
> >>> location lists. Any duplicate or overlapping ranges should be
> >>> coalesced down to a single range.
> >>>
> >>> For the testcase with current ToT you've got a single location of
> >>> DW_OP_fbreg + offset which seems to have absolutely no correlation
> >>> with the assembly. I'm somewhat confused as to what's going on here.
> >>> We've definitely got some problem here.
> >>>
> >>> -eric
> >>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131029/5598f637/attachment.html>