<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 12, 2013 at 6:18 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank" class="cremed">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">>> -    // In addition to making sure we produce it in every TU, we have to<br>

>> make<br>
>> -    // sure llvm keeps it.<br>
><br>
><br>
> This comment ^^^ seems to very clearly indicate that there is a need for<br>
> other TUs to be able to reference this alias. (The second part of the<br>
> comment that is...)<br>
<br>
</div>The issue is not that other TU reference it. It is that every comdat<br>
must be identical. Since we don't have direct support for comdats in<br>
the LLVM ir yet, what the old code was doing was using a weak_odr<br>
alias.<br>
<br>
The new code simply replaces anything that would use one of the<br>
linkonce_odr destructors with the one we know it is equivalent to.<br>
<div class="im"><br>
>><br>
>> -    // FIXME: Instead of outputting an alias we could just replace every<br>
>> use of<br>
>> -    // AliasDecl with TargetDecl.<br>
>> -    assert(Linkage == TargetLinkage);<br>
>> -    Linkage = llvm::GlobalValue::WeakODRLinkage;<br>
><br>
><br>
> And yet the code that replaces this:<br>
><br>
>> +    // Instead of creating as alias to a linkonce_odr, replace all of the<br>
>> uses<br>
>> +    // of the aliassee.<br>
>> +    if (TargetLinkage == llvm::GlobalValue::LinkOnceODRLinkage) {<br>
>> +      Replacements[MangledName] = Aliasee;<br>
>> +      return false;<br>
>> +    }<br>
><br>
><br>
> This doesn't preserve the same set of invariants the code above seems to<br>
> indicate are needed: the ability for there to be a weak ODR alias emitted<br>
> into this TU.<br>
><br>
> So, what's going on here? Any ideas? Hopefully Nick can add a test case when<br>
> the reduction finishes.<br>
<br>
</div>In hindsight the change to use a weak_odr alias to a linkonce_odr<br>
destructor was a bad hack around not having comdats. GCC always puts<br>
D1 and D2 in a comdat when they alias, but having direct comdat<br>
support means it can use a new name for the comdat, which avoid<br>
problems when mixing old and new object files.<br>
<br>
LLVM puts weak symbols in comdats, but we have no way of controlling<br>
the name and need hacks to make sure they are identical in every TU.<br>
This works when everything is using that clang or gcc, but can cause<br>
problems with .o files from different version of clang since the<br>
linker will see comdats with different symbols in them.<br>
<br>
Can you check if the comdats are identical in the link issue you are<br>
having?</blockquote><div><br></div><div>Not sure, Nick or Eric may be able to provide more details.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
What is the linker error? </blockquote><br>"warning: relocation refers to discarded section"<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Do any of the .o files were produced<br>
by clang [<br>
194000,r194095)?<br></blockquote><div><br></div><div>Not in a ready-for-test-case form.</div><div><br></div><div>This still seems like it is fundamentally breaking ABI with old clang versions, and potentially with GCC. I'm not sure we should do this, and I would like to not do this without some lead time to test it out. Certainly, it is currently blocking everything we do with Clang internally, literally everything is broken. =[ Can you at least go back to emitting the weak_odr alias so that link steps which rely on this continue to work?</div>
</div><br></div></div>