[PATCH v2] fix PowerPC < 7 w/ Altivec not to default to power7

[PATCH v2] fix PowerPC < 7 w/ Altivec not to default to power7

[Rene Rebe]

Hi Kewen,

v2 with test case - I hope I worked all your nits in:

Glibc uses .machine to determine assembler optimizations to use.
However, since reworking the rs6000 .machine output selection in
commit e154242724b084380e3221df7c08fcdbd8460674 22 May 2019, G5 as
well as Cell, and even power4 w/ -maltivec currently resulted in
power7. Mask _ALTIVEC away as the .machine selection already did for
GFX and GPOPT.

powerpc64-t2-linux-gnu-gcc  test.c -S -o - -mcpu=G5
	.file	"test.c"
	.machine power7
	.abiversion 2
	.section	".text"
	.ident	"GCC: (GNU) 10.2.0"
	.section	.note.GNU-stack,"",@progbits

We ship this in T2/Linux [2] since 2020 and it is tested on G5, Cell
and Power8.

Signed-of-by: René Rebe <rene@exactcode.de>

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97367
[2] https://t2sde.org

--- a/gcc/config/rs6000/rs6000.cc.vanilla	2021-04-25 22:57:16.964223106 +0200
+++ b/gcc/config/rs6000/rs6000.cc	2024-06-10 18:20:27.193223841 +0200
@@ -5765,7 +5765,8 @@
   HOST_WIDE_INT flags = rs6000_isa_flags;
 
   /* Disable the flags that should never influence the .machine selection.  */
-  flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ISEL);
+  flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ALTIVEC
+	      | OPTION_MASK_ISEL);
 
   if ((flags & (ISA_3_1_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0)
     return "power10";

--- a/gcc/testsuite/gcc.target/powerpc/pr97367.c.vanilla	2024-05-30 18:26:29.839784279 +0200
+++ b/gcc/testsuite/gcc.target/powerpc/pr97367.c	2024-10-06 18:20:34.873818482 +0200
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-mdejagnu-cpu=G5" } */
+
+int dummy ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler "power4" } } */

-- 
  René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin
  https://exactcode.com | https://t2sde.org | https://rene.rebe.de

Re: [PATCH v2] fix PowerPC < 7 w/ Altivec not to default to power7

[Segher Boessenkool]

Hi!

What does "powerpc < 7" mean?  Something before POWER ISA 2.06?

On Tue, Jun 11, 2024 at 04:22:54PM +0200, Rene Rebe wrote:
> Glibc uses .machine to determine assembler optimizations to use.

What does this mean?

.machine is an *output* for glibc; nothing in glibc reads source code.

Nothing the ".machine" directive does has anything to do with
optimisations.  Instead, it simply changes what architecture level is
used for the following code. what specific instructions are supported
mainly.

> --- a/gcc/testsuite/gcc.target/powerpc/pr97367.c.vanilla	2024-05-30 18:26:29.839784279 +0200
> +++ b/gcc/testsuite/gcc.target/powerpc/pr97367.c	2024-10-06 18:20:34.873818482 +0200
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mdejagnu-cpu=G5" } */
> +
> +int dummy ()
> +{
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler "power4" } } */

Please explain (in the testcase, not here!) what this is meant to test!

You probably want to say {\mpower4\M} instead, btw.  Unless you want to
match ".machine spower436" as well?


Segher

Re: [PATCH v2] fix PowerPC < 7 w/ Altivec not to default to power7

[René Rebe]

Hi!

> On Jun 12, 2024, at 00:15, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> Hi!
> 
> What does "powerpc < 7" mean?  Something before POWER ISA 2.06?

PowerPC ISA level 7 or whatever you like to call it.

> On Tue, Jun 11, 2024 at 04:22:54PM +0200, Rene Rebe wrote:
>> Glibc uses .machine to determine assembler optimizations to use.
> 
> What does this mean?
> 
> .machine is an *output* for glibc; nothing in glibc reads source code.

The glibc build with gcc since 2019 with -mcpu=g5, cell or anything before
power7 w/ altiven will use assembly optimizations with instructions not
supported by the CPU. I found out the hard way because the resultings
binaries threw SIGILL.

> Nothing the ".machine" directive does has anything to do with
> optimisations.  Instead, it simply changes what architecture level is
> used for the following code. what specific instructions are supported
> mainly.

I could probably go grep the glibc sources again 4 years later for you.

>> --- a/gcc/testsuite/gcc.target/powerpc/pr97367.c.vanilla 2024-05-30 18:26:29.839784279 +0200
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr97367.c 2024-10-06 18:20:34.873818482 +0200
>> @@ -0,0 +1,9 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-mdejagnu-cpu=G5" } */
>> +
>> +int dummy ()
>> +{
>> +  return 0;
>> +}
>> +
>> +/* { dg-final { scan-assembler "power4" } } */
> 
> Please explain (in the testcase, not here!) what this is meant to test!
> 
> You probably want to say {\mpower4\M} instead, btw.  Unless you want to
> match ".machine spower436" as well?

That sounds indeed reasonable. I guess we can make it match .machine, too.
Updated test-case welcome ;-)

	René

-- 
ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin
http://exactcode.com | http://exactscan.com | http://ocrkit.com

Re: [PATCH v2] fix PowerPC < 7 w/ Altivec not to default to power7

[René Rebe]

Hey,

> On Jun 12, 2024, at 00:27, René Rebe <rene@exactcode.de> wrote:
> 
> Hi!
> 
>> On Jun 12, 2024, at 00:15, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>> 
>> Hi!
>> 
>> What does "powerpc < 7" mean?  Something before POWER ISA 2.06?
> 
> PowerPC ISA level 7 or whatever you like to call it.
> 
>> On Tue, Jun 11, 2024 at 04:22:54PM +0200, Rene Rebe wrote:
>>> Glibc uses .machine to determine assembler optimizations to use.
>> 
>> What does this mean?
>> 
>> .machine is an *output* for glibc; nothing in glibc reads source code.
> 
> The glibc build with gcc since 2019 with -mcpu=g5, cell or anything before
> power7 w/ altiven will use assembly optimizations with instructions not
> supported by the CPU. I found out the hard way because the resultings
> binaries threw SIGILL.

Thankfully to total recall I actually debugged this live, 4 years ago on YouTube:

	https://www.youtube.com/watch?v=0gU5n3XhGOw

It is actually in glibc’s preconfigure explicitly grep’ing for it to choose
the submachine assembler optimizations:

preconfigure:case "${machine}:${submachine}" in
preconfigure:      | grep -E "mcpu=|.machine" -m 1 \
preconfigure:      | sed -e "s/.*machine //" -e "s/.*mcpu=\(.*\)\"/\1/“`

While we could argue that the glibc configure code is also not particularly
stelar, gcc should define the correct .machine ISA level like it did before the
quoted change in 2019 and my patch submitted nearly 4 years ago
fixes that ;-)

You can also support the work I’m doing daily over at:

	https://patreon.com/renerebe

Thank you so much,
	René

>> Nothing the ".machine" directive does has anything to do with
>> optimisations.  Instead, it simply changes what architecture level is
>> used for the following code. what specific instructions are supported
>> mainly.
> 
> I could probably go grep the glibc sources again 4 years later for you.
> 
>>> --- a/gcc/testsuite/gcc.target/powerpc/pr97367.c.vanilla 2024-05-30 18:26:29.839784279 +0200
>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr97367.c 2024-10-06 18:20:34.873818482 +0200
>>> @@ -0,0 +1,9 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-mdejagnu-cpu=G5" } */
>>> +
>>> +int dummy ()
>>> +{
>>> +  return 0;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "power4" } } */
>> 
>> Please explain (in the testcase, not here!) what this is meant to test!
>> 
>> You probably want to say {\mpower4\M} instead, btw.  Unless you want to
>> match ".machine spower436" as well?
> 
> That sounds indeed reasonable. I guess we can make it match .machine, too.
> Updated test-case welcome ;-)


-- 
ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin
http://exactcode.com | http://exactscan.com | http://ocrkit.com