| Message ID | 9a27584c-0ee3-b1f0-62eb-2fff0a487058@vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 26, 2017 at 10:33:48PM -0500, Peter Bergner wrote: > Tulio added support for two new AT_HWCAP2 bits to GLIBC which have been > recently added to the kernel: > > https://www.sourceware.org/ml/libc-alpha/2017-06/msg00069.html > > This patch adds support for them to the __builtin_cpu_supports() builtin > function so we can test for them. > > Tested on powerpc64le-linux with no regressions. Is this ok for trunk? Okay. Could we use a shared (with glibc) header, or reduce duplication some other way? Segher > * config/rs6000/ppc-auxv.h (PPC_FEATURE2_DARN): New define. > (PPC_FEATURE2_SCV): Likewise. > * config/rs6000/rs6000.c (cpu_supports_info): Use them. > > gcc/testsuite/ > * gcc.target/powerpc/cpu-builtin-1.c (darn, scv): Add tests.
On 6/27/17 10:51 AM, Segher Boessenkool wrote: > On Mon, Jun 26, 2017 at 10:33:48PM -0500, Peter Bergner wrote: >> Tulio added support for two new AT_HWCAP2 bits to GLIBC which have been >> recently added to the kernel: >> >> https://www.sourceware.org/ml/libc-alpha/2017-06/msg00069.html >> >> This patch adds support for them to the __builtin_cpu_supports() builtin >> function so we can test for them. >> >> Tested on powerpc64le-linux with no regressions. Is this ok for trunk? > > Okay. > > Could we use a shared (with glibc) header, or reduce duplication some > other way? Not really, since either GCC or GLIBC could be built with/against an older version of the other package and if that package were the one to have the defines, then we'd get a build error. Peter
On Tue, Jun 27, 2017 at 10:55:53AM -0500, Peter Bergner wrote: > On 6/27/17 10:51 AM, Segher Boessenkool wrote: > > On Mon, Jun 26, 2017 at 10:33:48PM -0500, Peter Bergner wrote: > >> Tulio added support for two new AT_HWCAP2 bits to GLIBC which have been > >> recently added to the kernel: > >> > >> https://www.sourceware.org/ml/libc-alpha/2017-06/msg00069.html > >> > >> This patch adds support for them to the __builtin_cpu_supports() builtin > >> function so we can test for them. > >> > >> Tested on powerpc64le-linux with no regressions. Is this ok for trunk? > > > > Okay. > > > > Could we use a shared (with glibc) header, or reduce duplication some > > other way? > > Not really, since either GCC or GLIBC could be built with/against an > older version of the other package and if that package were the one > to have the defines, then we'd get a build error. Not use an installed header, that's not what I'm asking. Share the source file, i.e., just copy it over from the glibc source tree (it should probably hold the master copy). Fewer typos, cannot forget to update some entry, etc. Segher
On 6/27/17 11:07 AM, Segher Boessenkool wrote: > Not use an installed header, that's not what I'm asking. Share the > source file, i.e., just copy it over from the glibc source tree (it > should probably hold the master copy). Fewer typos, cannot forget to > update some entry, etc. Ah, that's make sense. I'll have a look at how easy it is. In the mean time, I'll hold off on committing this. Peter
On 6/27/17 11:07 AM, Segher Boessenkool wrote: > Not use an installed header, that's not what I'm asking. Share the > source file, i.e., just copy it over from the glibc source tree (it > should probably hold the master copy). Fewer typos, cannot forget to > update some entry, etc. So the glibc file is: sysdeps/powerpc/bits/hwcap.h which contains only the #define PPC_FEATURE[2]_* definitions. The GCC file is: gcc/config/rs6000/ppc-auxv.h and contains the same #define's as hwcap.h above, plus the additional #defines's: /* The PLATFORM value stored in the TCB is offset by _DL_FIRST_PLATFORM. */ #define _DL_FIRST_PLATFORM 32 /* AT_PLATFORM bits. These must match the values defined in GLIBC. */ #define PPC_PLATFORM_POWER4 0 #define PPC_PLATFORM_PPC970 1 #define PPC_PLATFORM_POWER5 2 ... which match values in glibc's sysdeps/powerpc/dl-procinfo.h, but that file contains a lot more than just the defines that we (GCC) doesn't want or need. ppc-auxv.h also contains the following helper macros that calculate the fixed offsets to the TCB slots that glibc initializes, but glibc has access to the structs that the slows live in, so they don't need these helper macros and hence don't have them: /* Thread Control Block (TCB) offsets of the AT_PLATFORM, AT_HWCAP and AT_HWCAP2 values. These must match the values defined in GLIBC. */ #define TCB_PLATFORM_OFFSET ((TARGET_64BIT) ? -28764 : -28724) #define TCB_HWCAP_BASE_OFFSET ((TARGET_64BIT) ? -28776 : -28736) #define TCB_HWCAP1_OFFSET \ ((BYTES_BIG_ENDIAN) ? TCB_HWCAP_BASE_OFFSET : TCB_HWCAP_BASE_OFFSET+4) #define TCB_HWCAP2_OFFSET \ ((BYTES_BIG_ENDIAN) ? TCB_HWCAP_BASE_OFFSET+4 : TCB_HWCAP_BASE_OFFSET) #define TCB_HWCAP_OFFSET(ID) \ (((ID) == 0) ? TCB_HWCAP1_OFFSET : TCB_HWCAP2_OFFSET) These are only used in rs6000.c, so I could move them there. So given the above, how do we want to handle this? If we were to copy a header file(s) over from glibc, are we able to modify it in the process? Ie, to remove the parts we don't need like hwcap.h's use of: #if !defined(_SYS_AUXV_H) && !defined(_SYSDEPS_SYSDEP_H) # error "Never include <bits/hwcap.h> directly; use <sys/auxv.h> instead." #endif which would trigger for our use of it. And also to remove unneeded code from dl-procinfo.h, since we only want the #defines. Peter
Segher, any response to my findings below about whether we should try and share header files with GLIBC? Peter On 6/27/17 1:06 PM, Peter Bergner wrote: > On 6/27/17 11:07 AM, Segher Boessenkool wrote: >> Not use an installed header, that's not what I'm asking. Share the >> source file, i.e., just copy it over from the glibc source tree (it >> should probably hold the master copy). Fewer typos, cannot forget to >> update some entry, etc. > > So the glibc file is: > > sysdeps/powerpc/bits/hwcap.h > > which contains only the #define PPC_FEATURE[2]_* definitions. > The GCC file is: > > gcc/config/rs6000/ppc-auxv.h > > and contains the same #define's as hwcap.h above, plus the additional > #defines's: > > /* The PLATFORM value stored in the TCB is offset by _DL_FIRST_PLATFORM. */ > #define _DL_FIRST_PLATFORM 32 > > /* AT_PLATFORM bits. These must match the values defined in GLIBC. */ > #define PPC_PLATFORM_POWER4 0 > #define PPC_PLATFORM_PPC970 1 > #define PPC_PLATFORM_POWER5 2 > ... > > which match values in glibc's sysdeps/powerpc/dl-procinfo.h, but that > file contains a lot more than just the defines that we (GCC) doesn't > want or need. > > ppc-auxv.h also contains the following helper macros that calculate the > fixed offsets to the TCB slots that glibc initializes, but glibc has > access to the structs that the slows live in, so they don't need these > helper macros and hence don't have them: > > /* Thread Control Block (TCB) offsets of the AT_PLATFORM, AT_HWCAP and > AT_HWCAP2 values. These must match the values defined in GLIBC. */ > #define TCB_PLATFORM_OFFSET ((TARGET_64BIT) ? -28764 : -28724) > #define TCB_HWCAP_BASE_OFFSET ((TARGET_64BIT) ? -28776 : -28736) > #define TCB_HWCAP1_OFFSET \ > ((BYTES_BIG_ENDIAN) ? TCB_HWCAP_BASE_OFFSET : TCB_HWCAP_BASE_OFFSET+4) > #define TCB_HWCAP2_OFFSET \ > ((BYTES_BIG_ENDIAN) ? TCB_HWCAP_BASE_OFFSET+4 : TCB_HWCAP_BASE_OFFSET) > #define TCB_HWCAP_OFFSET(ID) \ > (((ID) == 0) ? TCB_HWCAP1_OFFSET : TCB_HWCAP2_OFFSET) > > These are only used in rs6000.c, so I could move them there. > > So given the above, how do we want to handle this? If we were to copy a > header file(s) over from glibc, are we able to modify it in the process? > Ie, to remove the parts we don't need like hwcap.h's use of: > > #if !defined(_SYS_AUXV_H) && !defined(_SYSDEPS_SYSDEP_H) > # error "Never include <bits/hwcap.h> directly; use <sys/auxv.h> instead." > #endif > > which would trigger for our use of it. And also to remove unneeded code from > dl-procinfo.h, since we only want the #defines. > > Peter > >
On Fri, Jun 30, 2017 at 11:53:48AM -0500, Peter Bergner wrote: > >> Not use an installed header, that's not what I'm asking. Share the > >> source file, i.e., just copy it over from the glibc source tree (it > >> should probably hold the master copy). Fewer typos, cannot forget to > >> update some entry, etc. [ snip ] We could factor out the parts used by both projects to a separate file, and share that one. Does glibc also have the names in our cpu_supports_info array? Should it have them? What I'm after is making errors and omissions less likely. Maybe I just shouldn't worry, it's not like auxv changes *that* often. Segher
Segher Boessenkool <segher@kernel.crashing.org> writes: > On Fri, Jun 30, 2017 at 11:53:48AM -0500, Peter Bergner wrote: >> >> Not use an installed header, that's not what I'm asking. Share the >> >> source file, i.e., just copy it over from the glibc source tree (it >> >> should probably hold the master copy). Fewer typos, cannot forget to >> >> update some entry, etc. >... > Does glibc also have the names in our cpu_supports_info array? It does have the names, but it isn't compatible with the cpu_supports_info array. > Should it have them? Maybe, but glibc is frozen now and we won't be able to make changes until glibc 2.26 is released.
On 6/27/17 10:51 AM, Segher Boessenkool wrote: > On Mon, Jun 26, 2017 at 10:33:48PM -0500, Peter Bergner wrote: >> Tulio added support for two new AT_HWCAP2 bits to GLIBC which have been >> recently added to the kernel: >> >> https://www.sourceware.org/ml/libc-alpha/2017-06/msg00069.html >> >> This patch adds support for them to the __builtin_cpu_supports() builtin >> function so we can test for them. >> >> Tested on powerpc64le-linux with no regressions. Is this ok for trunk? > > Okay. Thanks committed to trunk and as we discussed offline, also committed to the GCC 7 and GCC 6 release branches. Peter
Index: gcc/config/rs6000/ppc-auxv.h =================================================================== --- gcc/config/rs6000/ppc-auxv.h (revision 249611) +++ gcc/config/rs6000/ppc-auxv.h (working copy) @@ -89,6 +89,8 @@ #define PPC_FEATURE2_HTM_NOSC 0x01000000 #define PPC_FEATURE2_ARCH_3_00 0x00800000 #define PPC_FEATURE2_HAS_IEEE128 0x00400000 +#define PPC_FEATURE2_DARN 0x00200000 +#define PPC_FEATURE2_SCV 0x00100000 /* Thread Control Block (TCB) offsets of the AT_PLATFORM, AT_HWCAP and Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 249611) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -379,7 +379,9 @@ static const struct { "tar", PPC_FEATURE2_HAS_TAR, 1 }, { "vcrypto", PPC_FEATURE2_HAS_VEC_CRYPTO, 1 }, { "arch_3_00", PPC_FEATURE2_ARCH_3_00, 1 }, - { "ieee128", PPC_FEATURE2_HAS_IEEE128, 1 } + { "ieee128", PPC_FEATURE2_HAS_IEEE128, 1 }, + { "darn", PPC_FEATURE2_DARN, 1 }, + { "scv", PPC_FEATURE2_SCV, 1 } }; /* On PowerPC, we have a limited number of target clones that we care about Index: gcc/testsuite/gcc.target/powerpc/cpu-builtin-1.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/cpu-builtin-1.c (revision 249611) +++ gcc/testsuite/gcc.target/powerpc/cpu-builtin-1.c (working copy) @@ -62,4 +62,6 @@ use_cpu_supports_builtins (unsigned int p[35] = __builtin_cpu_supports ("ucache"); p[36] = __builtin_cpu_supports ("vcrypto"); p[37] = __builtin_cpu_supports ("vsx"); + p[38] = __builtin_cpu_supports ("darn"); + p[39] = __builtin_cpu_supports ("scv"); }