<div dir="ltr">Fair enough - carry on. Thanks for your patience/help :)</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Apr 20, 2017 at 11:41 AM Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On Apr 20, 2017, at 11:15 AM, Robinson, Paul <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>> wrote:<br>
><br>
><br>
><br>
>> -----Original Message-----<br>
>> From: <a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a> [mailto:<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>]<br>
>> Sent: Thursday, April 20, 2017 10:33 AM<br>
>> To: David Blaikie<br>
>> Cc: <a href="mailto:reviews%2BD30785%2Bpublic%2B750a0f0570653e04@reviews.llvm.org" target="_blank">reviews+D30785+public+750a0f0570653e04@reviews.llvm.org</a>; Robinson,<br>
>> Paul; <a href="mailto:clayborg@gmail.com" target="_blank">clayborg@gmail.com</a>; <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>; <a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a><br>
>> Subject: Re: [PATCH] D30785: [DWARF] Versioning for DWARF constants;<br>
>> verify FORMs<br>
>><br>
>><br>
>>> On Apr 20, 2017, at 10:20 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>>><br>
>>> Also, Adrian - still curious to hear if you have any thoughts about<br>
>> /where/ that check should go (see the review history discussing failing<br>
>> when the attribute is added, rather than later when the abbreviation is<br>
>> created) as well as how it should be implemented (assert V report fatal,<br>
>> etc)<br>
>><br>
>> IIUC the main concern about putting it into the Abbreviation creation is<br>
>> that it might be expensive to get at the DWARF version of the CU.  I'm not<br>
>> sure how expensive it is going to be, but if this is in an !NDEBUG path<br>
>> only, perhaps the extra cost can be justified. That said, if it turns out<br>
>> to add a noticeable delay to our RA stage2 bots, then the code as it is in<br>
>> this review now is the right trade-off.<br>
>><br>
>> -- adrian<br>
><br>
> A more significant concern was that it required (or at least, that's what<br>
> I came up with) virtual methods, which makes the DIE class bigger, which<br>
> could be a significant memory consumption problem.  For a debugging check,<br>
> that seems like too much cost (because it also is a cost in release mode).<br>
<br>
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.<br>
<br>
-- adrian<br>
<br>
> --paulr<br>
<br>
</blockquote></div>