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

Wed Aug 22 17:18:17 PDT 2018

Yes, we are. Sorry about the delayed response.

On Mon, Aug 20, 2018 at 8:31 AM Louis Dionne <ldionne at apple.com> wrote:

> Nice! Just to confirm, if you are using the candidate Clang-7, you are
> building with `-D_LIBCPP_HIDE_FROM_ABI_PER_TU=1`, right?
>
> Louis
>
> On Aug 20, 2018, at 11:28, Erik van der Poel <erikv at google.com> wrote:
>
> 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/20180822/37904417/attachment.html>