<div dir="ltr">LGTM. (I'm not an expert on the comdat feature but based on your process of following LangRef when writing the test I assume it covers the needed features).<div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 3, 2015 at 9:19 AM, Vedant Kumar <span dir="ltr"><<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I've responded to comments inline -- two patches attached.<br>
<span class=""><br>
>><br>
>> -* The bitcode format produced by a X.Y release will be readable by all following<br>
>> -  X.Z releases and the (X+1).0 release.<br>
>> +* Additions and changes to the IR should be reflected in<br>
>> +  ``test/Bitcode/compatibility.ll``.<br>
>> +<br>
>> +* The bitcode format produced by a X.Y release will be readable by all<br>
>> +  following X.Z releases and the (X+1).0 release. To help ensure this, an X.Y<br>
>> +  version of ``test/Bitcode/compatibility.ll`` should be assembled and<br>
>> +  committed after each release.<br>
>><br>
>> Please be a bit more specific here. I would like us to document explicitly the naming of the assembled files to avoid guesswork in the future.<br>
<br>
</span>Here's the new language:<br>
<br>
> 508 * Additions and changes to the IR should be reflected in<br>
> 509   ``test/Bitcode/compatibility.ll``.<br>
> 510<br>
> 511 * The bitcode format produced by a X.Y release will be readable by all<br>
> 512   following X.Z releases and the (X+1).0 release.<br>
> 513<br>
> 514 * After each X.Y release, ``compatibility.ll`` must be copied to<br>
> 515   ``compatibility-X.Y.ll``. The corresponding bitcode file should be assembled<br>
> 516   using the X.Y build and committed as ``compatibility-X.Y.ll.bc``.<br>
<span class=""><br>
<br>
<br>
>> +@comdat.samesize = global i32 0, comdat<br>
>> +; CHECK: @comdat.samesize = global i32 0, comdat<br>
>><br>
>> Is there a reason this is not covering usage of comdats with explicit comdat name?<br>
<br>
</span>Not a good one. At one point I had mistakenly assumed that 3.6 lacked support for this.<br>
<br>
``compatibility.ll`` has coverage for this, so I've copied it into the 3.6 test.<br>
<br>
[1] fix-combat-coverage.patch<br>
<br>
[Bitcode] Cover explicit comdat names in compatibility-3.6.ll<br>
<br>
[2] fix-devpolicy-language.patch<br>
<br>
[docs] Language about managing compatibility.ll made clearer<br>
<br>
 </blockquote></div><br></div>