[PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[H.J. Lu]

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(+)

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 */
-- 
2.9.3

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[Zack Weinberg]

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

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[Zack Weinberg]

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

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[H.J. Lu]

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.

-- 
H.J.

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[Zack Weinberg]

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

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[Zack Weinberg]

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

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

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.

>>> 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



-- 
H.J.

[-- Attachment #2: 0001-Add-__BEGIN_DECLS-and-__END_DECLS-for-C.patch --]
[-- Type: text/x-patch, Size: 2127 bytes --]

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

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Add __BEGIN_DECLS and __END_DECLS for C++

[Zack Weinberg]

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