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

[Adrian Prantl]

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.

-- 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
>>>