[PATCH] D30785: [DWARF] Versioning for DWARF constants; verify FORMs

[David Blaikie via llvm-commits]

Fair enough - carry on. Thanks for your patience/help :)

On Thu, Apr 20, 2017 at 11:41 AM Adrian Prantl <aprantl at apple.com> wrote:

>
> > On Apr 20, 2017, at 11:15 AM, Robinson, Paul <paul.robinson at sony.com>
> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: aprantl at apple.com [mailto:aprantl at apple.com]
> >> Sent: Thursday, April 20, 2017 10:33 AM
> >> To: David Blaikie
> >> Cc: reviews+D30785+public+750a0f0570653e04 at reviews.llvm.org; Robinson,
> >> Paul; clayborg at gmail.com; llvm-commits at lists.llvm.org;
> nhaehnle at gmail.com
> >> Subject: Re: [PATCH] D30785: [DWARF] Versioning for DWARF constants;
> >> verify FORMs
> >>
> >>
> >>> On Apr 20, 2017, at 10:20 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>
> >>> Also, Adrian - still curious to hear if you have any thoughts about
> >> /where/ that check should go (see the review history discussing failing
> >> when the attribute is added, rather than later when the abbreviation is
> >> created) as well as how it should be implemented (assert V report fatal,
> >> etc)
> >>
> >> IIUC the main concern about putting it into the Abbreviation creation is
> >> that it might be expensive to get at the DWARF version of the CU.  I'm
> not
> >> sure how expensive it is going to be, but if this is in an !NDEBUG path
> >> only, perhaps the extra cost can be justified. That said, if it turns
> out
> >> to add a noticeable delay to our RA stage2 bots, then the code as it is
> in
> >> this review now is the right trade-off.
> >>
> >> -- adrian
> >
> > A more significant concern was that it required (or at least, that's what
> > I came up with) virtual methods, which makes the DIE class bigger, which
> > could be a significant memory consumption problem.  For a debugging
> check,
> > that seems like too much cost (because it also is a cost in release
> mode).
>
> What I've heard so far doesn't make this option sound very appealing.
> Since this is a debugging check only that extra cost isn't justified. Let's
> just do it as the current patch does it.
>
> -- adrian
>
> > --paulr
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170420/5266fea9/attachment.html>