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

[Adrian Prantl via llvm-commits]

> 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