r184276 - Debug Info: support for gdwarf-2 gdwarf-3 gdwarf-4

[Eric Christopher]

I see you went ahead and committed these (the mailing list is down,
but I saw the updates via phabricator).

The discussion may have faltered, but given that there was
disagreement on the original patch I would think that it's your
responsibility to ping the thread before committing.

I've spoken with Dan again and we've chatted about the module flag
versus in the compile unit and the rationale for the direction you've
chosen. Overall while I think it should be the other way it's not
overly damaging to llvm for it to be the direction that you've already
written the code and can always be changed later if we find it too
limiting.

That said, I do believe that the warning should go away and that you
should fix the code (I guess during module linking since you've gone
this direction) so that it sorts out the values and re-sets the module
flag to the minimum of all modules. This will mean that a number of
more advanced features may get turned off, but it's the conservative
answer.

-eric



On Thu, Jun 20, 2013 at 4:48 PM, Manman Ren <mren at apple.com> wrote:
>
> On Jun 19, 2013, at 4:28 PM, Eric Christopher wrote:
>
>>>> I don't think a module flag is the way we want to go about this, I
>>>> think it should be an attribute on the compile unit created.
>>>
>>> Hmm, I could've sworn we (Eric, Dan, and myself) in person a couple of
>>> weeks ago & decided that a module flag was the way to go - but I'm not
>>> 100% sure that's how the conversation concluded & I'm also not sure I
>>> can reconstruct all the arguments.
>>>
>>> I think it went something like: we'll need the module-level
>>> functionality anyway, and it'll be able to report
>>> errors/incompatibilities in a uniform way that users will be used to,
>>> whereas if we did this down in debug info we wouldn't be able to use
>>> such error reporting if there are "incompatible" dwarf version
>>> requests. (I suppose there might not be - in which case it'd be "min"
>>> or "max" essentially and even then, presumably, having the common
>>> module functionality able to handle that for us seems reasonable/nice)
>>>
>>
>> Right, but that pretty much contradicts how linkers deal with them
>> today - it's perfectly compatible to link them without warning.
>>
>>>> Few constraints there:
>>>>
>>>> a) this means that we just check the compile unit as we create up the
>>>> DIE and we don't have to check the module.
>>>
>>> I'm not sure that's a huge change - and we'd still have to have some
>>> code resolve the difference between different modules & presumably
>>> rewrite the compile unit of each one to bring it up to whatever the
>>> One True dwarf version is that we'll be using for this build, no? Or
>>> are you expecting to generate some DWARF-N for some compile units in
>>> the same compilation and DWARF-M for others?
>>
>> Only at LTO time and only up to a minimum of 3. Otherwise, I'm
>> perfectly fine with generating differing values of dwarf for each
>> compile unit.
>>
>>> I'm not sure how we settled on this issue - whether the module
>>> handling should just bump everything up to the "minimum compatible"
>>> version (floor of DWARF-4 or whatever, otherwise the max of all the
>>> specified versions from the different CUs).
>>>
>>>> c) at LTO time we'll likely want to constrain to a minimum dwarf
>>>> version that contains ref addr so that we can minimize, but this is by
>>>> no means required,
>>>
>>> Fair enough, so maybe the 'floor' above isn't required.
>>>
>>
>> *nod* Basically the way we're doing LTO is going to require
>> inter-compile unit references. Other than that there's no real need to
>> do any munging of the value at all. Hence me figuring to just leave it
>> on each compile unit and keep the logic on what to emit in the compile
>> unit itself.
>>
>> It probably comes down to simple preference really there's nothing
>> compelling about either way unless we want to explicitly warn people
>> and that'd be creating a new warning for "little" reason and it might
>> be surprising for people that were previously getting no warnings with
>> normal compilation moving to LTO. That said, maybe they'll be
>> surprised when they get inter-compile unit references.
> Any conclusion here? I am okay with either choice, and about to modify the backend.
>
> Manman
>
>>
>> *shrug*
>>
>> -eric
>>
>>
>>>> d) it matches how linkers actually link debug information
>>>>
>>>> Also:
>>>>
>>>>> +  else if (Opts.getDebugInfo() != CodeGenOptions::NoDebugInfo)
>>>>> +    // Default Dwarf version is 3 if we are generating debug information.
>>>>> +    Opts.DwarfVersion = 3;
>>>>>
>>>>
>>>> We want 4 here I believe, or even a level for "latest" with all of the
>>>> bells and whistles. 4 is a likely sufficient start though since we'd
>>>> have to define what "all the bells and whistles" means :) Or, as an
>>>> alternate question, "why 3?"
>>>>
>>>> Thanks!
>>>>
>>>> -eric
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>