| Message ID | 20170511141836.GB26086@intel.com |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 64113 invoked by alias); 11 May 2017 14:19:05 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 64073 invoked by uid 89); 11 May 2017 14:19:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, NO_DNS_FOR_FROM, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mga02.intel.com X-ExtLoop1: 1 Date: Thu, 11 May 2017 07:18:36 -0700 From: "H.J. Lu" <hongjiu.lu@intel.com> To: GNU C Library <libc-alpha@sourceware.org> Subject: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++ Message-ID: <20170511141836.GB26086@intel.com> Reply-To: "H.J. Lu" <hjl.tools@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.8.0 (2017-02-23) |
Commit Message
Lu, Hongjiu
May 11, 2017, 2:18 p.m. UTC
Add __BEGIN_DECLS and __END_DECLS to support C++. Any comments? H.J. -- * include/ifunc-impl-list.h: Add __BEGIN_DECLS and __END_DECLS. --- include/ifunc-impl-list.h | 4 ++++ 1 file changed, 4 insertions(+)
Comments
On Thu, May 11, 2017 at 10:18 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: > Add __BEGIN_DECLS and __END_DECLS to support C++. > > Any comments? Could you please explain why you found this to be necessary? ifunc-impl-list.h is an internal header which should never be compiled as C++ in the first place. zw
On Thu, May 11, 2017 at 7:29 AM, Zack Weinberg <zackw@panix.com> wrote: > On Thu, May 11, 2017 at 10:18 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> Add __BEGIN_DECLS and __END_DECLS to support C++. >> >> Any comments? > > Could you please explain why you found this to be necessary? > ifunc-impl-list.h is an internal header which should never be compiled > as C++ in the first place. I am integrating memcpy_benchmark.cc: https://gist.github.com/ekelsen/b66cc085eb39f0495b57679cdb1874fa into glibc benchtests. It is a C++ program.
On Thu, May 11, 2017 at 10:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, May 11, 2017 at 7:29 AM, Zack Weinberg <zackw@panix.com> wrote: >> On Thu, May 11, 2017 at 10:18 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>> Add __BEGIN_DECLS and __END_DECLS to support C++. >>> >>> Any comments? >> >> Could you please explain why you found this to be necessary? >> ifunc-impl-list.h is an internal header which should never be compiled >> as C++ in the first place. > > I am integrating memcpy_benchmark.cc: > > https://gist.github.com/ekelsen/b66cc085eb39f0495b57679cdb1874fa > > into glibc benchtests. It is a C++ program. This program does not appear to need ifunc-impl-list.h. Please elaborate. zw
On Thu, May 11, 2017 at 7:43 AM, Zack Weinberg <zackw@panix.com> wrote: > On Thu, May 11, 2017 at 10:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, May 11, 2017 at 7:29 AM, Zack Weinberg <zackw@panix.com> wrote: >>> On Thu, May 11, 2017 at 10:18 AM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>> Add __BEGIN_DECLS and __END_DECLS to support C++. >>>> >>>> Any comments? >>> >>> Could you please explain why you found this to be necessary? >>> ifunc-impl-list.h is an internal header which should never be compiled >>> as C++ in the first place. >> >> I am integrating memcpy_benchmark.cc: >> >> https://gist.github.com/ekelsen/b66cc085eb39f0495b57679cdb1874fa >> >> into glibc benchtests. It is a C++ program. > > This program does not appear to need ifunc-impl-list.h. Please elaborate. > Please see hjl/x86/optimize branch in glibc git repo.
On Thu, May 11, 2017 at 10:45 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, May 11, 2017 at 7:43 AM, Zack Weinberg <zackw@panix.com> wrote: >> >> This program does not appear to need ifunc-impl-list.h. Please elaborate. > > Please see hjl/x86/optimize branch in glibc git repo. I don't especially appreciate being made to dig through a bunch of code I'm unfamiliar with. It would have been easy for you to write "The existing benchtests framework uses ifunc-impl-list.h to iterate over all ifunc implementations of a particular string function. This works as long as the test program is C, but I want to integrate a third-party benchmark <url> written in C++, so I need to make ifunc-impl-list.h C++-safe". If that had accompanied the original patch it would have been better all around. It looks to me as if IFUNC_IMPL_ADD is not C++-safe and cannot easily be made so, so I don't like this change. What prevents you from rewriting the third-party benchmark in C, since you have to modify it anyway? It's not doing anything that is difficult in plain C. zw
On Thu, May 11, 2017 at 7:57 AM, Zack Weinberg <zackw@panix.com> wrote: > On Thu, May 11, 2017 at 10:45 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, May 11, 2017 at 7:43 AM, Zack Weinberg <zackw@panix.com> wrote: >>> >>> This program does not appear to need ifunc-impl-list.h. Please elaborate. >> >> Please see hjl/x86/optimize branch in glibc git repo. > > I don't especially appreciate being made to dig through a bunch of > code I'm unfamiliar with. It would have been easy for you to write > "The existing benchtests framework uses ifunc-impl-list.h to iterate > over all ifunc implementations of a particular string function. This > works as long as the test program is C, but I want to integrate a > third-party benchmark <url> written in C++, so I need to make > ifunc-impl-list.h C++-safe". If that had accompanied the original > patch it would have been better all around. > > It looks to me as if IFUNC_IMPL_ADD is not C++-safe and cannot easily IFUNC_IMPL_ADD is only used in ifunc-impl-list.c, which is the part of libc and in C. > be made so, so I don't like this change. What prevents you from > rewriting the third-party benchmark in C, since you have to modify it > anyway? It's not doing anything that is difficult in plain C. > I'd to preserve the original benchmark as much as possible so that little is lost, comparing with the original one.
On Thu, May 11, 2017 at 11:10 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, May 11, 2017 at 7:57 AM, Zack Weinberg <zackw@panix.com> wrote: >> On Thu, May 11, 2017 at 10:45 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, May 11, 2017 at 7:43 AM, Zack Weinberg <zackw@panix.com> wrote: >>>> >>>> This program does not appear to need ifunc-impl-list.h. Please elaborate. >>> >>> Please see hjl/x86/optimize branch in glibc git repo. >> >> I don't especially appreciate being made to dig through a bunch of >> code I'm unfamiliar with. It would have been easy for you to write >> "The existing benchtests framework uses ifunc-impl-list.h to iterate >> over all ifunc implementations of a particular string function. This >> works as long as the test program is C, but I want to integrate a >> third-party benchmark <url> written in C++, so I need to make >> ifunc-impl-list.h C++-safe". If that had accompanied the original >> patch it would have been better all around. >> >> It looks to me as if IFUNC_IMPL_ADD is not C++-safe and cannot easily > > IFUNC_IMPL_ADD is only used in ifunc-impl-list.c, which is the > part of libc and in C. Can IFUNC_IMPL_ADD be removed from ifunc-impl-list.h then? I would be okay with adding __BEGIN_DECLS/__END_DECLS to the header as long as all of the code within it -- including macros -- was safe for use from C++ programs, and with a comment explaining that the header may get used from C++ benchmarks. >> be made so, so I don't like this change. What prevents you from >> rewriting the third-party benchmark in C, since you have to modify it >> anyway? It's not doing anything that is difficult in plain C. > > I'd to preserve the original benchmark as much as possible so that > little is lost, comparing with the original one. That's fair. zw
diff --git a/include/ifunc-impl-list.h b/include/ifunc-impl-list.h index 22ca05f..f4be574 100644 --- a/include/ifunc-impl-list.h +++ b/include/ifunc-impl-list.h @@ -22,6 +22,8 @@ #include <stdbool.h> #include <stddef.h> +__BEGIN_DECLS + struct libc_ifunc_impl { /* The name of function to be tested. */ @@ -53,4 +55,6 @@ extern size_t __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array, size_t max); +__END_DECLS + #endif /* ifunc-impl-list.h */