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

[Eric Christopher]

On Tue, Jul 2, 2013 at 5:45 PM, Manman Ren <mren at apple.com> wrote:
>
> On Jul 2, 2013, at 5:40 PM, Eric Christopher wrote:
>
>> I see you went ahead and committed these (the mailing list is down,
>> but I saw the updates via fabricator).
>
> I did ping once, and didn't get any reply. And it seems that nobody has
> strong opinion on one over the other.
>
> If we want to change it, it is not that hard to do :)
>

In the long run I think we'll end up changing it. Please do implement
the functionality that I requested though.

Thanks.

-eric

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