[PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Peter Bergner]

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?

Peter

gcc/
	* 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.

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");
 }

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Segher Boessenkool]

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.

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Peter Bergner]

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

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Segher Boessenkool]

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

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Peter Bergner]

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

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Peter Bergner]

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

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Peter Bergner]

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

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Segher Boessenkool]

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

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Tulio Magno Quites Machado Filho]

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.

-- 
Tulio Magno

Re: [PATCH, rs6000] Add support to __builtin_cpu_supports() for two new HWCAP2 bits

[Peter Bergner]

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