On 2014 May 7, at 17:48, Justin Bogner <[email protected]> wrote:
> "Duncan P. N. Exon Smith" <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
