[cfe-dev] [libc++] A proposal for getting rid of __always_inline__ in _LIBCPP_INLINE_VISIBILITY

Mon Aug 20 08:28:38 PDT 2018

Thank you for doing this, Louis! Our large stack frame problem has gone
away.

On Thu, Jul 12, 2018 at 8:13 AM Louis Dionne via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

>
>
> On Jul 10, 2018, at 19:42, Louis Dionne via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>
> On Jul 10, 2018, at 17:25, Louis Dionne via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>
> On Jul 10, 2018, at 16:46, Eric Fiselier <eric at efcs.ca> wrote:
>
> On Tue, Jul 10, 2018 at 1:13 PM, Louis Dionne <ldionne at apple.com> wrote:
>
>>
>> On Jul 10, 2018, at 10:07, Eric Fiselier <eric at efcs.ca> wrote:
>>
>> […]
>>
>>
>>
>>> This would look something like:
>>>
>>>     #ifdef _LIBCPP_NO_INTERNAL_LINKAGE_FOR_ABI_UNSTABLE_SYMBOLS
>>>     #   define _LIBCPP_HIDE_FROM_ABI
>>> __attribute__((__visibility__("hidden")))
>>>     #else
>>>     #   define _LIBCPP_HIDE_FROM_ABI
>>> __attribute__((__visibility__("hidden"),internal_linkage))
>>>     #endif
>>>
>>> In the future, we can decide which default behavior we want, but for
>>> now, I suggest we stick to what we have right now, which is support for
>>> both (1) and (2). It would be fine to change this in the future if we make
>>> that decision.
>>>
>>>
>> When we don’t have internal linkage, I suspect we'll want to keep
>> `__always_inline__` as to prevent static libraries from providing each
>> other with incompatible function definitions (I think this could occur?).
>>
>>
>> The idea here is that some people don’t care about interoperability of
>> static libraries (or, equivalently, object files) built with different
>> libc++ headers. And since they don’t care about this, they’d rather not pay
>> the cost of inlining or internal linkage, which is code bloat.
>>
>
> The `__always_inline__` is always needed, at least when mixing ABI stable
> and ABI hidden symbols in a explicitly instantiated template. Here's a
> reproducer that should fail to link on GCC and Clang:
> https://godbolt.org/g/LLZGjG
>
>
> I don’t believe this example is conforming. Per
> http://eel.is/c++draft/temp.explicit#12, `__init` should still be
> instantiated in `main.o`, because `__init` is marked inline. I must be
> wrong because otherwise both GCC and Clang are compiling this wrong.
>
>
> I’ve kicked off a thread on llvm-dev here to try to clarify the situation:
>  http://lists.llvm.org/pipermail/llvm-dev/2018-July/124549.html
>
> I want to avoid at all costs getting stuck like all the previous times
> this topic was brought up, so let me scale down the proposal to something
> that will still relieve much of the pain, while letting space for
> improvement in the future. The new proposal would be:
>
> 1. Everything that is ABI stable is marked with a visibility macro like
> today (no change here).
> 2. Everything that is marked with `_LIBCPP_INLINE_VISIBILITY` today is
> marked with a new `_LIBCPP_HIDE_FROM_ABI` macro instead. This macro is
> defined to `__attribute__((__visibility__("hidden"), internal_linkage))`,
> or to `__attribute__((__visibility__("hidden"), __always_inline__))` if the
> `internal_linkage` attribute is not supported (e.g. GCC). There is no way
> to turn off both `internal_linkage` and `__always_inline__`, not until the
> issue brought up above is resolved. This would look something like this:
>
>     #if __has_attribute(internal_linkage)
>     #  define _LIBCPP_INTERNAL_LINKAGE __attribute__ ((internal_linkage))
>     #else
>     #  define _LIBCPP_INTERNAL_LINKAGE __attribute__ ((__always_inline__))
>     #endif
>
>     #ifndef _LIBCPP_HIDE_FROM_ABI
>     #  define _LIBCPP_HIDE_FROM_ABI
> __attribute__((__visibility__(“hidden”))) _LIBCPP_INTERNAL_LINKAGE
>     #endif
>
> In the future, as a pure improvement, we can add an alternative definition
> of `_LIBCPP_HIDE_FROM_ABI` which uses neither `internal_linkage` nor
> `__always_inline__`, for those that don't care about static archive
> interoperability but care about code size.
>
> 3. We still don't touch `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY` for
> now.
>
> Concretely, this will solve the problem of abusive inlining of the vast
> majority of functions whenever libc++ is compiled on Clang, which is
> already a big improvement. And then I can go back to the design board to
> solve the remaining issues, having at least made a some progress.
>
> What do you think?
>
>
> I’ve put up a patch implementing this scaled down proposal for review:
> https://reviews.llvm.org/D49240. Once/If we go forward with this, I’m
> committed to explore ways of solving the remaining problems, which are
> (please LMK if you see something else):
>
> 1. `_LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY` still uses
> `__always_inline__` and we’d like to get rid of this.
> 2. We’d like the ability to only specify hidden visibility, but let
> ODR-equivalent functions be deduplicated across TUs. This makes it unsafe
> to mix TUs built with different versions of libc++, but has the potential
> of saving on code size.
>
> Louis
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180820/815fd4a4/attachment.html>