[cfe-dev] libc++ and large stack frames

[James Y Knight via cfe-dev]

On Mon, Jun 4, 2018 at 6:56 PM Duncan P. N. Exon Smith via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

>
> On Jun 4, 2018, at 14:57, Erik van der Poel <erikv at google.com> wrote:
>
> On Mon, Jun 4, 2018 at 2:11 PM Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>>
>>
>> On Jun 4, 2018, at 13:52, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> On Mon, 4 Jun 2018, 21:43 Duncan P. N. Exon Smith via cfe-dev, <
>> cfe-dev at lists.llvm.org> wrote:
>>
>>>
>>>
>>> On May 31, 2018, at 15:35, Richard Smith via cfe-dev <
>>> cfe-dev at lists.llvm.org> wrote:
>>>
>>> On 31 May 2018 at 14:38, Friedman, Eli via cfe-dev <
>>> cfe-dev at lists.llvm.org> wrote:
>>>
>>>> On 5/31/2018 1:07 PM, Erik van der Poel via cfe-dev wrote:
>>>>
>>>>> This is a proposal to add a configuration option to disable
>>>>> _LIBCPP_INLINE_VISIBILITY, which causes large stack frames in non-optimized
>>>>> builds because it uses the always_inline attribute to force significant
>>>>> amounts of inlining of libc++ code.
>>>>>
>>>>> The new configuration option could be called
>>>>> _LIBCPP_DISABLE_INLINE_VISIBILITY.
>>>>>
>>>>> Note that _LIBCPP_INLINE_VISIBILITY and _LIBCPP_ALWAYS_INLINE have
>>>>> identical definitions. One could be renamed to the other or both could be
>>>>> renamed to _LIBCPP_HIDE_FROM_ABI as part of this work.
>>>>>
>>>>
>>> I think it's useful to have different names for these, since they have
>>> different intent.  It would be reasonable to change the definition of one
>>> without changing the other.
>>>
>>> Renaming _LIBCPP_INLINE_VISIBILITY to _LIBCPP_HIDE_FROM_ABI makes sense
>>> to me though.
>>>
>>> One option we'd like to investigate (but haven't yet) is changing
>>> _LIBCPP_INLINE_VISIBILITY/_LIBCPP_HIDE_FROM_ABI to drop the "always_inline"
>>> but keep the "hidden".  Have you considered
>>> _LIBCPP_DISABLE_ALWAYS_INLINE_IN_INLINE_VISIBILITY (or equivalent)?  If
>>> not, why not?
>>>
>>
>> IIRC that results in missing definitions for all of the explicitly
>> instantiated template members, because the versions in the DSO are not
>> visible to their consumers.
>>
>>
>> It seems wrong to have any explicitly instantiated template members with
>> _LIBCPP_INLINE_VISIBILITY.  Do we really do that?
>>
>
> Removing always_inline from _LIBCPP_INLINE_VISIBILITY was actually the
> first thing I tried. I got errors like these (from ld):
>
> hidden symbol
> '_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEpLEPKc' is
> not defined locally
>
>
> Okay, I see; we're opting specific symbols out of living in the dylib.
> Once you remove the always_inline, the `extern template` promises that
> these symbols will be in the libc++.dylib, but they're not actually
> available there...
>
> I don't think we should give up on hiding these symbols from the ABI
> though, and adding weak external symbols is not a great outcome.  I'd
> rather we add an attribute (straw man: "no_extern_template") that opts
> these symbols out of any `extern template` declaration.
>
> Then, depending on a macro setting, we pick between:
> - __attribute__((hidden,always_inline))
> - __attribute__((hidden,no_extern_template))
>

This surely affects only a tiny minority of the symbols currently marked
always_inline, right? Could we start by removing it from all the functions
which aren't affected by the _LIBCPP_EXTERN_TEMPLATE or
_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS, first, to get the largest volume of stuff
fixed, before trying to deal with these problematic edge cases?

Also -- is there really an actual benefit of hiding these symbols from the
export list? Once you're exporting any significant functions from a
nontrivial class like basic_string<char>, I'm not clear as to how it's of
real benefit to hide other functions like operator+=. I'd think that
typically everything generated by the explicit template instantiation
should just be exported in future versions.

Of course there is the Apple-platform issue of requiring compatibility of
apps compiled with new libc++ headers to be able to run against an old
libc++ shared library. When adding new functions to an existing template
class, those functions won't be present in the older shared lib version.
ISTM that for that case, it'd be entirely reasonable to mark them
internal_linkage (or whatever) only when targeting old platforms.


>
Also, I think we can use semantic names for the macros.
> - _LIBCPP_HIDE_FROM_ABI to apply the attributes (replacing
> _LIBCPP_INLINE_VISIBILITY, as you suggested).
> - _LIBCPP_DISABLE_HIDE_FROM_ABI_FROM_OBJECTS (to switch from always_inline
> (maybe, eventually, internal_linkage) to no_extern_template).
> _______________________________________________
> 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/20180606/e75cfbfd/attachment.html>