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

[David Blaikie]

On Jul 2, 2013 5:40 PM, "Eric Christopher" <echristo at gmail.com> wrote:
>
> 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

Could we try to capture some/more of the rationale in both directions
actually in email rather than (or at least in addition to) offline
conversations so we can refer to it later. As it stands it seems were still
lacking any of that conversation in an archived form.

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

What's the reasoning for this choice?it would seem to me that the maximum
would make a fair bit of sense, though I admit it does seem somewhat
arbitrary either way.

> 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
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130703/870d6ea7/attachment.html>