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

[Eric Christopher]

On Tue, Oct 29, 2013 at 1:10 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
>
> On Tue, Oct 29, 2013 at 1:06 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>> On Oct 29, 2013, at 11:51, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> >
>> >
>> >
>> > 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.
>> >
>>
>> That would be a fairly trivial fix in
>> DWARFDebug::createScopeChildrenDIE(). I’m wondering: Should there ever be
>> more than one DIE describing a variable in different lexical scopes or
>> should there be only ever a single DIE with multiple entries in the
>> DW_AT_location list?
>
>
> The latter, I'm fairly sure.
>

Yes.

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131029/de934c94/attachment.html>