[llvm-dev] [cfe-dev] [RFC] __attribute__((internal_linkage))
Mehdi Amini via llvm-dev
llvm-dev at lists.llvm.org
Tue Jul 12 18:38:22 PDT 2016
Hi Evgenii,
I have one question about this (planned) change: what if a function is not inlined? The linker will not ODR merge them with this change, which isn’t great.
What makes “internal” linkage more desirable than "linkonce_odr + visibility hidden"?
—
Mehdi
> On Jul 12, 2016, at 6:16 PM, Evgenii Stepanov via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>
> Hi,
>
> it's been put on hold for other higher priority work. I still want to
> get it done one day, but it's not clear when that would be.
>
> On Tue, Jul 12, 2016 at 6:11 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
>> Hi Evgenii,
>>
>> I was wondering what the status is of your work to attach "internal_linkage"
>> to methods of standard library classes in libc++. The following piece of
>> code doesn't link because a symbol (string::empty) is undefined and it
>> sounds like your work might fix the linkage error I'm seeing.
>>
>> $ cat test1.cpp
>> #include <string>
>> #include <functional>
>>
>> int main() {
>> using namespace std::placeholders;
>> auto b = std::bind(&std::string::empty, _1);
>> }
>>
>> On Mon, Nov 9, 2015 at 1:58 PM, Evgenii Stepanov via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>>>
>>> In fact, during the code review we've decided against allowing this
>>> attribute on namespaces: http://reviews.llvEm.org/D13925
>>>
>>>
>>> The intended use of this attribute in libc++ is on functions that
>>> programs are not allowed to take address of, like all methods of
>>> standard library classes. Visibility("hidden") attribute (which we are
>>> replacing with internal_linkage) violates ODR the same way.
>>>
>>> Also, this attribute (for us) is not about locality or performance.
>>> Its primary purpose is to reliably exclude a class method from the
>>> export table. This is the way libc++ manages ABI stability: all class
>>> methods that are not part of the library ABI are defined in the
>>> headers with a set of attributes that ensure that they become internal
>>> to the translation unit. Always_inline does that in almost all cases,
>>> but almost is not good enough. Internal_linkage is a straighforward
>>> way to achieve the same.
>>>
>>>
>>> On Mon, Nov 9, 2015 at 1:34 PM, Martin J. O'Riordan
>>> <martin.oriordan at movidius.com> wrote:
>>>> With respect only to '__attribute__((internal_linkage))', not 'nodebug'
>>>> and other parts of this topic; does hiding "some" members and not others not
>>>> introduce a violation of the ODR because some members of the class as it
>>>> appears in one translation unit are not the same actual definitions as the
>>>> apparently "same" members of the class in another translation unit? In
>>>> particular the ODR requirement that would ensure that the address of a
>>>> method was the same regardless of where the address was taken, including
>>>> 'static' methods (no 'this' pointer)? Or hidden things like the guard
>>>> variables for local static objects, default argument initialisers (which are
>>>> instanced), etc.?
>>>>
>>>> I must admit that having spent years trying to lock down the definition
>>>> of the ODR, that this seems to fly in the opposite direction of the intent
>>>> of that rule.
>>>>
>>>> I can (sort of) see why for particular reasons of locality you might
>>>> want multiple actual definitions, even though they might exhibit the same
>>>> behaviour, but maybe the answer for locality optimisation would be for the
>>>> linker technology to evolve so that multiple copies of the same definition
>>>> (sic) could be made; perhaps even at the Basic Block level [Block Level
>>>> Linking]. But this too would lie under the auspices of the "as if" rule and
>>>> respect the ODR.
>>>>
>>>> There is something I am missing in this that is not making me at all
>>>> comfortable about the use of this attribute for namespaces (apart from the
>>>> reopening issues raised by others), and in particular the application to
>>>> parts of a class but not the whole class. I just feel that this is leading
>>>> to semantic trouble.
>>>>
>>>> Thanks,
>>>>
>>>> MartinO
>>>>
>>>> -----Original Message-----
>>>> From: Evgenii Stepanov [mailto:eugeni.stepanov at gmail.com]
>>>> Sent: 09 November 2015 18:49
>>>> To: Robinson, Paul <Paul_Robinson at playstation.sony.com>
>>>> Cc: Chris Lattner <clattner at apple.com>;
>>>> sstewartgallus00 at mylangara.bc.ca; llvm-dev at lists.llvm.org; Martin J.
>>>> O'Riordan <martin.oriordan at movidius.com>
>>>> Subject: Re: [llvm-dev] [cfe-dev] [RFC]
>>>> __attribute__((internal_linkage))
>>>>
>>>> Sorry, I totally missed updates to this thread.
>>>>
>>>> Anonymous namespace does not work for libc++ because we need to hide
>>>> some class members, and export the rest. Namespace would hide all of them.
>>>> AFAIK, "static" is un-deprecated in C++11 for multiple reasons, including
>>>> this one.
>>>>
>>>> I agree that nodebug should not affect codegen. It would also kill debug
>>>> info (ex. all debug locations in libc++ headers).
>>>>
>>>>
>>>> On Mon, Nov 2, 2015 at 11:57 AM, Robinson, Paul via llvm-dev
>>>> <llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
>>>>>> Chris Lattner via llvm-dev
>>>>>> Sent: Saturday, October 31, 2015 2:58 PM
>>>>>> To: sstewartgallus00 at mylangara.bc.ca
>>>>>> Cc: LLVM Developers
>>>>>> Subject: Re: [llvm-dev] [cfe-dev] [RFC]
>>>>>> __attribute__((internal_linkage))
>>>>>>
>>>>>> Hi Stewart,
>>>>>>
>>>>>> I saw this get brought up at the LLVM devmtg in the context of
>>>>>> improving the stack size of libc++ frames.
>>>>>>
>>>>>> Have you guys considered a different approach to solving this
>>>>>> problem? In the case of libc++, the _LIBCPP_INLINE_VISIBILITY macro
>>>>>> currently expands out to "__attribute__ ((__always_inline__))”. It
>>>>>> seems reasonable to me to have it also add the “nodebug” attribute as
>>>>>> well. This should allow the allocas generated by inlining lots of
>>>>>> frames to be promoted to registers (because there is no debug info to
>>>>>> pessimize).
>>>>>
>>>>> Are you suggesting that 'nodebug' should affect code generation?
>>>>> It most definitely should not...
>>>>> --paulr
>>>>>
>>>>>>
>>>>>> This would dramatically shrink the stack frames of code using libc++
>>>>>> at - O0, and would also make it go a lot faster.
>>>>>>
>>>>>> -Chris
>>>>>>
>>>>>>
>>>>>>> On Oct 29, 2015, at 6:35 AM, Martin J. O'Riordan via llvm-dev
>>>>>>> <llvm-
>>>>>> dev at lists.llvm.org> wrote:
>>>>>>>
>>>>>>> I haven't been able to figure out from this thread why this
>>>>>>> attribute is
>>>>>> necessary at all.
>>>>>>>
>>>>>>> Anonymous or unnamed namespaces were added to C++ for this very
>>>>>>> purpose,
>>>>>> but the ISO C++ Standard does not discuss "linkage" per-se because it
>>>>>> is outside the scope of the language specification. But it does
>>>>>> describes it in terms of having a hidden name which is "unique" to
>>>>>> the translation unit, and under the "as if" rule, internal linkage is
>>>>>> a common convention for achieving this.
>>>>>>>
>>>>>>> This is just Standardese for dealing with what compiler
>>>>>>> implementers
>>>>>> typically handle as "internal linkage" anyway; at the same time, the
>>>>>> use of 'static' for this purpose was deprecated. This is
>>>>>> specifically stated in C++98 section 7.3.1.1 and unnamed namespaces
>>>>>> are still similarly defined in C++17 (Working Paper) although the
>>>>>> specific reference to the deprecation of 'static' for this purpose is
>>>>>> now gone.
>>>>>>>
>>>>>>> The closest the Standard gets to talking about linkage is with
>>>>>>> Linkage
>>>>>> Specifications, and even here it tries to avoid to avoid treading on
>>>>>> the definitions things like internal versus external linkage.
>>>>>>>
>>>>>>> So I am curious, what does this proposed attribute on namespaces
>>>>>>> achieve
>>>>>> that cannot already be achieved with an anonymous or unnamed
>>>>>> namespace?
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Martin O'Riordan - Movidius Ltd.
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: cfe-dev [mailto:cfe-dev-bounces at lists.llvm.org] On Behalf Of
>>>>>> Evgenii Stepanov via cfe-dev
>>>>>>> Sent: 23 October 2015 22:41
>>>>>>> To: Steven Stewart-Gallus <sstewartgallus00 at mylangara.bc.ca>
>>>>>>> Cc: Clang Dev <cfe-dev at lists.llvm.org>
>>>>>>> Subject: Re: [cfe-dev] [RFC] __attribute__((internal_linkage))
>>>>>>>
>>>>>>> Sounds right. The proposed attribute is a rough equivalent of
>>>>>>>
>>>>>>> static void foo::do_method(void)
>>>>>>> {
>>>>>>> // implementation here
>>>>>>> }
>>>>>>>
>>>>>>> where "static" does not mean a static class member, but makes the
>>>>>> declaration local to the translation unit. C-style "static".
>>>>>>>
>>>>>>> The patch in http://reviews.llvm.org/D13925 (waiting for review,
>>>>>>> btw!)
>>>>>> supports this attribute on classes and namespaces, with this syntax:
>>>>>>>
>>>>>>> namespace __attribute__((internal_linkage)) { }
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Oct 23, 2015 at 9:03 AM, Steven Stewart-Gallus via cfe-dev
>>>>>>> <cfe-
>>>>>> dev at lists.llvm.org> wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Can I ask for some clarification?
>>>>>>>>
>>>>>>>> Apparently, C++ doesn't allow to static class methods?
>>>>>>>>
>>>>>>>> While in C one might write inside a header file:
>>>>>>>>
>>>>>>>> static inline void mylib_foo_do_method(struct mylib_foo *foo) {
>>>>>>>> // implementation here
>>>>>>>> }
>>>>>>>>
>>>>>>>> In C++ the typical style would be to write something like:
>>>>>>>>
>>>>>>>> namespace mylib {
>>>>>>>>
>>>>>>>> void foo::do_method(void)
>>>>>>>> {
>>>>>>>> // implementation here
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> Unfortunately, the C++ case then implies some linkage behaviour
>>>>>>>> that some might not want.
>>>>>>>>
>>>>>>>> Apparently, one can't do things like:
>>>>>>>>
>>>>>>>> namespace mylib {
>>>>>>>>
>>>>>>>> static void foo::do_method(void)
>>>>>>>> {
>>>>>>>> // implementation here
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> Or,
>>>>>>>>
>>>>>>>> namespace {
>>>>>>>> namespace mylib {
>>>>>>>>
>>>>>>>> static void foo::do_method(void)
>>>>>>>> {
>>>>>>>> // implementation here
>>>>>>>> }
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> Also, if you wanted to an attribute to a whole namespace you
>>>>>>>> should do something like the following I think:
>>>>>>>>
>>>>>>>> namespace mylib {
>>>>>>>> [[clang::internal_linkage]];
>>>>>>>>
>>>>>>>> static void foo::do_method(void)
>>>>>>>> {
>>>>>>>> // implementation here
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Steven Stewart-Gallus
>>>>>>>> _______________________________________________
>>>>>>>> cfe-dev mailing list
>>>>>>>> cfe-dev at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>>>> _______________________________________________
>>>>>>> cfe-dev mailing list
>>>>>>> cfe-dev at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>
>>>>>> _______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
More information about the llvm-dev
mailing list