Message ID | CAMe9rOqQENp_6qpj9LpT6d1n0SvcP19Em_O8p0hb5MDeNWnn4A@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Thu, May 11, 2017 at 8:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, May 11, 2017 at 8:20 AM, Zack Weinberg <zackw@panix.com> wrote: >> 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. > > IFUNC_IMPL_ADD is used in > > sysdeps/arm/armv7/multiarch/ifunc-impl-list.c > sysdeps/i386/i686/multiarch/ifunc-impl-list.c > sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c > sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c > sysdeps/s390/multiarch/ifunc-impl-list.c > sysdeps/sparc/sparc64/multiarch/ifunc-impl-list.c > sysdeps/x86_64/multiarch/ifunc-impl-list.c > > Removing it from include/ifunc-impl-list.h isn't appropriate. Here is > the updated patch with comments. > Ping.
On 05/19/2017 01:57 PM, H.J. Lu wrote: > On Thu, May 11, 2017 at 8:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, May 11, 2017 at 8:20 AM, Zack Weinberg <zackw@panix.com> wrote: >>> 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. >> >> IFUNC_IMPL_ADD is used in >> >> sysdeps/arm/armv7/multiarch/ifunc-impl-list.c >> sysdeps/i386/i686/multiarch/ifunc-impl-list.c >> sysdeps/powerpc/powerpc32/power4/multiarch/ifunc-impl-list.c >> sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c >> sysdeps/s390/multiarch/ifunc-impl-list.c >> sysdeps/sparc/sparc64/multiarch/ifunc-impl-list.c >> sysdeps/x86_64/multiarch/ifunc-impl-list.c >> >> Removing it from include/ifunc-impl-list.h isn't appropriate. Here is >> the updated patch with comments. >> > > Ping. I am going to look into compiling the benchmarks under _ISOMAC. Please wait. zw
From 5eb2c187b50fdc0849f60a966fbc0ff0f407fd3d Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Wed, 10 May 2017 16:02:56 -0700 Subject: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++ Add __BEGIN_DECLS and __END_DECLS to support C++. IFUNC_IMPL_ADD and IFUNC_IMPL are used internally in libc. They shouldn't be used in any programs. * include/ifunc-impl-list.h: Add __BEGIN_DECLS and __END_DECLS. (IFUNC_IMPL_ADD, IFUNC_IMPL): Define only if __cplusplus isn't defined. --- include/ifunc-impl-list.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/include/ifunc-impl-list.h b/include/ifunc-impl-list.h index 22ca05f..7d53f11 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. */ @@ -32,20 +34,25 @@ struct libc_ifunc_impl bool usable; }; +#ifndef __cplusplus +/* NB: IFUNC_IMPL_ADD and IFUNC_IMPL are used internally in libc. They + shouldn't be used in any programs. */ + /* Add an IFUNC implementation, IMPL, for function FUNC, to ARRAY with USABLE at index I and advance I by one. */ -#define IFUNC_IMPL_ADD(array, i, func, usable, impl) \ +# define IFUNC_IMPL_ADD(array, i, func, usable, impl) \ extern __typeof (func) impl attribute_hidden; \ (array)[i++] = (struct libc_ifunc_impl) { #impl, (void (*) (void)) impl, (usable) }; /* Return the number of IFUNC implementations, N, for function FUNC if string NAME matches FUNC. */ -#define IFUNC_IMPL(n, name, func, ...) \ +# define IFUNC_IMPL(n, name, func, ...) \ if (strcmp (name, #func) == 0) \ { \ __VA_ARGS__; \ return n; \ } +#endif /* __cplusplus */ /* Fill ARRAY of MAX elements with IFUNC implementations for function NAME and return the number of valid entries. */ @@ -53,4 +60,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 */ -- 2.9.3