2017-03-29 20:30 GMT-07:00 Duncan P. N. Exon Smith <dexonsm...@apple.com>:
>
> On Mar 29, 2017, at 20:16, Eric Fiselier <e...@efcs.ca> wrote:
>
>
>
> On Wed, Mar 29, 2017 at 9:00 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>> Why are we propagating the use of always_inline?
>>
>
> The intent of this change wasn't to propagate or remove always_inline from
> any functions. It was to fix incompatible dylibs built by GCC.
> The way it did that is by removing the __attribute__((visibility("hidden")))
> part while building the dylib, because GCC was ignoring the
> `visibility("default")` on the extern template declarations for basic
> string.
>
>
>> In other places, we use always_inline to avoid affecting ABI (a
>> visibility hack). But you're not removing basic_string from the dylib here.
>>
>
> That's a very good point. It's likely we should convert many of the
> `_LIBCPP_ALWAYS_INLINE`s to inline instead and the compiler should
> do the right thing. However that change is orthogonal to this one, and
> hence it wasn't made here.
>
>
> Reading comprehension issue on my part. I think probably replied to the
> wrong commit. Bisection pointed at r278356, but I saw that that commit was
> reverted and I couldn't find where it finally landed. So then I did a
> careless git-blame for what I thought was the relevant line and ended up
> here.
>
> Also I think the doc for _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY is
> misleading. I'll try to update it in the coming days.
>
>
>
>
>> It has major downsides:
>> - It causes inlining at -O0, which causes major regressions in debugging
>> experiences.
>> - It causes inlining at -O0, which, without other optimizations, can blow
>> up stack size of static_initializers.
>>
>
> For some extra context: we tracked down a stack overflow to the related
> changes to add always_inline to basic_string::__init. Our user had some
> string => enum value map declared statically, like in the attached
> stackoverflow.cpp. At -Os (and higher), there's no difference. But with
> -O0 and -O1, they started getting a stack overflow at launch.
>
That's not a bug but a feature ;-)
It was a good way to show the user he's not doing the right thing by using
a costly initialization routine!
--
Mehdi
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
