r208258 - CodeGen: Don't set hidden visibility on symbols with local linkage

[Duncan P. N. Exon Smith]

On 2014 May 7, at 17:48, Justin Bogner <mail at justinbogner.com> wrote:

> "Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
>> --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed May  7 17:36:11 2014
>> @@ -1586,7 +1586,8 @@ ItaniumCXXABI::getOrCreateThreadLocalWra
>>                 CGM.getLLVMLinkageVarDefinition(VD, /*isConstant=*/false)),
>>       WrapperName.str(), &CGM.getModule());
>>   // Always resolve references to the wrapper at link time.
>> -  Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
>> +  if (!Wrapper->hasLocalLinkage())
>> +    Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
>>   return Wrapper;
>> }
> 
> Does the comment here need updating?

I'm not sure.  Maybe it should be moved inside the `if`?

IIUC, the idea of `hidden` is that the next link is the only one that
gets to "see" a symbol.  I think the idea of this comment is that all
references to the symbol have to be resolved by the next link.  For
`internal` symbols, the linker never gets to see the symbol at all...
so the references must have *already* been resolved.  The comment
seems okay to me as is.

If you can think of better text for the comment, let me know and I'll
change it :).

FYI, for those that missed it, this was a prelude to a larger patch
series reviewed on llvm-commits [1].

[1]: http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/186407