[libcxx] r282345 - Use __attribute__((internal_linkage)) when available.
Mehdi Amini via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 12 21:55:59 PDT 2016
Thanks Eric!
> On Oct 12, 2016, at 9:18 PM, Eric Fiselier <eric at efcs.ca> wrote:
>
> Sure, I've reverted the change in r284101.
>
> I'll try and respond in detail tomorrow.
>
> /Eric
>
> On Wed, Oct 12, 2016 at 9:57 PM, Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> Hi Eric,
>
> This is not clear to me that this patch is correct as-is.
>
> See the last message in the thread: http://lists.llvm.org/pipermail/cfe-dev/2016-July/049985.html <http://lists.llvm.org/pipermail/cfe-dev/2016-July/049985.html>
>
> I think we should reach a consensus on the right course of actions about this first.
>
> —
> Mehdi
>
>
>
>
>> On Sep 24, 2016, at 8:14 PM, Eric Fiselier via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>
>> Author: ericwf
>> Date: Sat Sep 24 22:14:13 2016
>> New Revision: 282345
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=282345&view=rev <http://llvm.org/viewvc/llvm-project?rev=282345&view=rev>
>> Log:
>> Use __attribute__((internal_linkage)) when available.
>>
>> Summary:
>> This patch has been a long time coming (Thanks @eugenis). It changes `_LIBCPP_INLINE_VISIBILITY` to use `__attribute__((internal_linkage))` instead of `__attribute__((visibility("hidden"), always_inline))`.
>>
>> The point of `_LIBCPP_INLINE_VISIBILITY` is to prevent inline functions from being exported from both the libc++ library and from user libraries. This helps libc++ better manage it's ABI.
>> Previously this was done by forcing inlining and modifying the symbols visibility. However inlining isn't guaranteed and symbol visibility only affects shared libraries making this an imperfect solution. `internal_linkage` improves this situation by making all symbols local to the TU they are emitted in, regardless of inlining or visibility. IIRC the effect of applying `__attribute__((internal_linkage))` to an inline function is the same as applying `static`.
>>
>> For more information about the attribute see: http://lists.llvm.org/pipermail/cfe-dev/2015-October/045580.html <http://lists.llvm.org/pipermail/cfe-dev/2015-October/045580.html>
>>
>> Most of the work for this patch was done by @eugenis.
>>
>>
>> Reviewers: mclow.lists, eugenis
>>
>> Subscribers: eugenis, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D24642 <https://reviews.llvm.org/D24642>
>>
>> Modified:
>> libcxx/trunk/include/__config
>> libcxx/trunk/src/string.cpp
>>
>> Modified: libcxx/trunk/include/__config
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=282345&r1=282344&r2=282345&view=diff <http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=282345&r1=282344&r2=282345&view=diff>
>> ==============================================================================
>> --- libcxx/trunk/include/__config (original)
>> +++ libcxx/trunk/include/__config Sat Sep 24 22:14:13 2016
>> @@ -34,6 +34,7 @@
>> #endif
>>
>> #if defined(_LIBCPP_ABI_UNSTABLE) || _LIBCPP_ABI_VERSION >= 2
>> +#define _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION 2
>> // Change short string representation so that string data starts at offset 0,
>> // improving its alignment in some cases.
>> #define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
>> @@ -49,6 +50,7 @@
>> #define _LIBCPP_ABI_FIX_UNORDERED_CONTAINER_SIZE_TYPE
>> #define _LIBCPP_ABI_VARIADIC_LOCK_GUARD
>> #elif _LIBCPP_ABI_VERSION == 1
>> +#define _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION 1
>> // Feature macros for disabling pre ABI v1 features. All of these options
>> // are deprecated.
>> #if defined(__FreeBSD__)
>> @@ -629,11 +631,19 @@ namespace std {
>> #endif
>>
>> #ifndef _LIBCPP_INLINE_VISIBILITY
>> -#define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__visibility__("hidden"), __always_inline__))
>> +# if __has_attribute(__internal_linkage__)
>> +# define _LIBCPP_INLINE_VISIBILITY __attribute__((__internal_linkage__, __always_inline__))
>> +# else
>> +# define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__visibility__("hidden"), __always_inline__))
>> +# endif
>> #endif
>>
>> #ifndef _LIBCPP_ALWAYS_INLINE
>> -#define _LIBCPP_ALWAYS_INLINE __attribute__ ((__visibility__("hidden"), __always_inline__))
>> +# if __has_attribute(__internal_linkage__)
>> +# define _LIBCPP_ALWAYS_INLINE __attribute__((__internal_linkage__, __always_inline__))
>> +# else
>> +# define _LIBCPP_ALWAYS_INLINE __attribute__ ((__visibility__("hidden"), __always_inline__))
>> +# endif
>> #endif
>>
>> #ifndef _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY
>>
>> Modified: libcxx/trunk/src/string.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/string.cpp?rev=282345&r1=282344&r2=282345&view=diff <http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/string.cpp?rev=282345&r1=282344&r2=282345&view=diff>
>> ==============================================================================
>> --- libcxx/trunk/src/string.cpp (original)
>> +++ libcxx/trunk/src/string.cpp Sat Sep 24 22:14:13 2016
>> @@ -29,6 +29,29 @@ template
>> string
>> operator+<char, char_traits<char>, allocator<char> >(char const*, string const&);
>>
>> +// These external instantiations are required to maintain dylib compatibility
>> +// for ABI v1 when using __attribute__((internal_linkage)) as opposed to
>> +// __attribute__((visibility("hidden"), always_inline)).
>> +#if _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION == 1
>> +template basic_string<char>::iterator
>> +basic_string<char>::insert(basic_string<char>::const_iterator,
>> + char const *, char const *);
>> +
>> +template basic_string<wchar_t>::iterator
>> +basic_string<wchar_t>::insert(basic_string<wchar_t>::const_iterator,
>> + wchar_t const *, wchar_t const *);
>> +
>> +template basic_string<char> &
>> +basic_string<char>::replace(basic_string<char>::const_iterator,
>> + basic_string<char>::const_iterator,
>> + char const *, char const *);
>> +
>> +template basic_string<wchar_t> &
>> +basic_string<wchar_t>::replace(basic_string<wchar_t>::const_iterator,
>> + basic_string<wchar_t>::const_iterator,
>> + wchar_t const *, wchar_t const *);
>> +#endif // _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION
>> +
>> namespace
>> {
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161012/9acfbfc3/attachment-0001.html>
More information about the cfe-commits
mailing list