diff mbox series

rs6000: Fix cpu selection w/ isel (PR100108)

Message ID 8bd3b0c11cb08f63d2ca41a21596d16635626661.1618930768.git.segher@kernel.crashing.org
State New
Headers show
Series rs6000: Fix cpu selection w/ isel (PR100108) | expand

Commit Message

Segher Boessenkool April 20, 2021, 3 p.m. UTC 
There are various non-IBM CPUs with isel as well, so it is easiest if we
just don't consider that flag here (it is not needed).

2021-04-20  Segher Boessenkool  <segher@kernel.crashing.org>

	PR target/100108
	* config/rs6000/rs6000.c (rs6000_machine_from_flags): Do not consider
	OPTION_MASK_ISEL.
---
Committed to trunk and 11.  Will do 10 in a week or so.


Segher


 gcc/config/rs6000/rs6000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Segher Boessenkool May 10, 2021, 10:04 p.m. UTC | #1 
On Tue, Apr 20, 2021 at 03:00:42PM +0000, Segher Boessenkool wrote:
> There are various non-IBM CPUs with isel as well, so it is easiest if we
> just don't consider that flag here (it is not needed).
> 
> 2021-04-20  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	PR target/100108
> 	* config/rs6000/rs6000.c (rs6000_machine_from_flags): Do not consider
> 	OPTION_MASK_ISEL.
> ---
> Committed to trunk and 11.  Will do 10 in a week or so.

Done now.


Segher
Sebastian Huber Jan. 11, 2022, 8:10 a.m. UTC | #2 
Hello Segher,

On 20/04/2021 17:00, Segher Boessenkool wrote:
> There are various non-IBM CPUs with isel as well, so it is easiest if we
> just don't consider that flag here (it is not needed).
> 
> 2021-04-20  Segher Boessenkool<segher@kernel.crashing.org>
> 
> 	PR target/100108
> 	* config/rs6000/rs6000.c (rs6000_machine_from_flags): Do not consider
> 	OPTION_MASK_ISEL.
> ---
> Committed to trunk and 11.  Will do 10 in a week or so.

sorry for the late response, however, I noticed a GCC build issue with 
this patch:

/tmp/sh/b-gcc-powerpc-rtems6/./gcc/xgcc 
-B/tmp/sh/b-gcc-powerpc-rtems6/./gcc/ -nostdinc 
-B/tmp/sh/b-gcc-powerpc-rtems6/powerpc-rtems6/m8540/nof/newlib/ -isystem 
/tmp/sh/b-gcc-powerpc-rtems6/powerpc-rtems6/m8540/nof/newlib/targ-include -isystem 
/home/EB/sebastian_h/src/gcc/newlib/libc/include 
-B/tmp/sh/i-powerpc-rtems6/powerpc-rtems6/bin/ 
-B/tmp/sh/i-powerpc-rtems6/powerpc-rtems6/lib/ -isystem 
/tmp/sh/i-powerpc-rtems6/powerpc-rtems6/include -isystem 
/tmp/sh/i-powerpc-rtems6/powerpc-rtems6/sys-include  -mcpu=8540 
-msoft-float -g -O2 -O2 
-I/home/EB/sebastian_h/src/gcc/libgcc/../newlib/libc/sys/rtems/include 
-g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -W -Wall -Wno-narrowing 
-Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes 
-Wold-style-definition  -isystem ./include   -g -DIN_LIBGCC2 
-fbuilding-libgcc -fno-stack-protector -Dinhibit_libc  -I. -I. 
-I../../../.././gcc -I/home/EB/sebastian_h/src/gcc/libgcc 
-I/home/EB/sebastian_h/src/gcc/libgcc/. 
-I/home/EB/sebastian_h/src/gcc/libgcc/../gcc 
-I/home/EB/sebastian_h/src/gcc/libgcc/../include  -DHAVE_CC_TLS  -o 
_negvdi2.o -MT _negvdi2.o -MD -MP -MF _negvdi2.dep -DL_negvdi2 -c 
/home/EB/sebastian_h/src/gcc/libgcc/libgcc2.c -fvisibility=hidden 
-DHIDE_EXPORTS
/tmp/ccZJ18fW.s: Assembler messages:
/tmp/ccZJ18fW.s:24: Error: unrecognized opcode: `isel'
make: *** [Makefile:501: _negvdi2.o] Error 1

The assembler is called like this:

/tmp/sh/b-gcc-powerpc-rtems6/./gcc/as -I 
/home/EB/sebastian_h/src/gcc/libgcc/../newlib/libc/sys/rtems/include -I 
. -I . -I ../../../.././gcc -I /home/EB/sebastian_h/src/gcc/libgcc -I 
/home/EB/sebastian_h/src/gcc/libgcc/. -I 
/home/EB/sebastian_h/src/gcc/libgcc/../gcc -I 
/home/EB/sebastian_h/src/gcc/libgcc/../include -a32 -me500 -mbig -o 
_negvdi2.o _negvdi2.s

Using -me500 seems to be all right, however, the file contains a machine 
directive:

	.file	"libgcc2.c"
	.machine ppc
	.section	".text"

If I remove the ".machine ppc" by hand, the file can be assembled with 
the above command line.

The affect of the patch is:

diff -u _negvdi2.s.before _negvdi2.s.after
--- _negvdi2.s.before   2022-01-11 09:07:43.313828636 +0100
+++ _negvdi2.s.after    2022-01-11 08:54:08.424946502 +0100
@@ -1,5 +1,5 @@
         .file   "libgcc2.c"
-       .machine power9
+       .machine ppc
         .section        ".text"
  .Ltext0:
         .align 2
Sebastian Huber Jan. 17, 2022, 8:06 p.m. UTC | #3 
On 11/01/2022 09:10, Sebastian Huber wrote:
> Hello Segher,
> 
> On 20/04/2021 17:00, Segher Boessenkool wrote:
>> There are various non-IBM CPUs with isel as well, so it is easiest if we
>> just don't consider that flag here (it is not needed).
>>
>> 2021-04-20  Segher Boessenkool<segher@kernel.crashing.org>
>>
>>     PR target/100108
>>     * config/rs6000/rs6000.c (rs6000_machine_from_flags): Do not consider
>>     OPTION_MASK_ISEL.
>> ---
>> Committed to trunk and 11.  Will do 10 in a week or so.
> 
> sorry for the late response, however, I noticed a GCC build issue with 
> this patch:
> 
> /tmp/sh/b-gcc-powerpc-rtems6/./gcc/xgcc 
> -B/tmp/sh/b-gcc-powerpc-rtems6/./gcc/ -nostdinc 
> -B/tmp/sh/b-gcc-powerpc-rtems6/powerpc-rtems6/m8540/nof/newlib/ -isystem 
> /tmp/sh/b-gcc-powerpc-rtems6/powerpc-rtems6/m8540/nof/newlib/targ-include -isystem 
> /home/EB/sebastian_h/src/gcc/newlib/libc/include 
> -B/tmp/sh/i-powerpc-rtems6/powerpc-rtems6/bin/ 
> -B/tmp/sh/i-powerpc-rtems6/powerpc-rtems6/lib/ -isystem 
> /tmp/sh/i-powerpc-rtems6/powerpc-rtems6/include -isystem 
> /tmp/sh/i-powerpc-rtems6/powerpc-rtems6/sys-include  -mcpu=8540 
> -msoft-float -g -O2 -O2 
> -I/home/EB/sebastian_h/src/gcc/libgcc/../newlib/libc/sys/rtems/include 
> -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -W -Wall -Wno-narrowing 
> -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes 
> -Wold-style-definition  -isystem ./include   -g -DIN_LIBGCC2 
> -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc  -I. -I. 
> -I../../../.././gcc -I/home/EB/sebastian_h/src/gcc/libgcc 
> -I/home/EB/sebastian_h/src/gcc/libgcc/. 
> -I/home/EB/sebastian_h/src/gcc/libgcc/../gcc 
> -I/home/EB/sebastian_h/src/gcc/libgcc/../include  -DHAVE_CC_TLS  -o 
> _negvdi2.o -MT _negvdi2.o -MD -MP -MF _negvdi2.dep -DL_negvdi2 -c 
> /home/EB/sebastian_h/src/gcc/libgcc/libgcc2.c -fvisibility=hidden 
> -DHIDE_EXPORTS
> /tmp/ccZJ18fW.s: Assembler messages:
> /tmp/ccZJ18fW.s:24: Error: unrecognized opcode: `isel'
> make: *** [Makefile:501: _negvdi2.o] Error 1
> 
> The assembler is called like this:
> 
> /tmp/sh/b-gcc-powerpc-rtems6/./gcc/as -I 
> /home/EB/sebastian_h/src/gcc/libgcc/../newlib/libc/sys/rtems/include -I 
> . -I . -I ../../../.././gcc -I /home/EB/sebastian_h/src/gcc/libgcc -I 
> /home/EB/sebastian_h/src/gcc/libgcc/. -I 
> /home/EB/sebastian_h/src/gcc/libgcc/../gcc -I 
> /home/EB/sebastian_h/src/gcc/libgcc/../include -a32 -me500 -mbig -o 
> _negvdi2.o _negvdi2.s
> 
> Using -me500 seems to be all right, however, the file contains a machine 
> directive:
> 
>      .file    "libgcc2.c"
>      .machine ppc
>      .section    ".text"
> 
> If I remove the ".machine ppc" by hand, the file can be assembled with 
> the above command line.
> 
> The affect of the patch is:
> 
> diff -u _negvdi2.s.before _negvdi2.s.after
> --- _negvdi2.s.before   2022-01-11 09:07:43.313828636 +0100
> +++ _negvdi2.s.after    2022-01-11 08:54:08.424946502 +0100
> @@ -1,5 +1,5 @@
>          .file   "libgcc2.c"
> -       .machine power9
> +       .machine ppc
>          .section        ".text"
>   .Ltext0:
>          .align 2
> 
> 

I can try to fix this, however, for me it is not really clear in which 
direction this should be fixed. I can build the GCC 10.3.0 release (it 
uses .machine power9). The GCC 10 branch is broken (it uses .machine 
ppc). Using the .machine directive and a command line option (-me500) 
seems to be a bit inconsistent. It should be one or the other.

Would a patch which changes .machine ppc to .machine e500 for -mcpu=8540 
be the right way to fix this issue?
Segher Boessenkool Jan. 17, 2022, 8:21 p.m. UTC | #4 
Hi!

On Mon, Jan 17, 2022 at 09:06:25PM +0100, Sebastian Huber wrote:
> On 11/01/2022 09:10, Sebastian Huber wrote:
> >On 20/04/2021 17:00, Segher Boessenkool wrote:
> >>There are various non-IBM CPUs with isel as well, so it is easiest if we
> >>just don't consider that flag here (it is not needed).
> >>
> >>2021-04-20  Segher Boessenkool<segher@kernel.crashing.org>
> >>
> >>    PR target/100108
> >>    * config/rs6000/rs6000.c (rs6000_machine_from_flags): Do not 
> >>consider
> >>    OPTION_MASK_ISEL.
> >>---
> >>Committed to trunk and 11.  Will do 10 in a week or so.
> >
> >sorry for the late response, however, I noticed a GCC build issue with 
> >this patch:

[ big snip ]

> >/tmp/ccZJ18fW.s: Assembler messages:
> >/tmp/ccZJ18fW.s:24: Error: unrecognized opcode: `isel'
> >make: *** [Makefile:501: _negvdi2.o] Error 1
> >
> >The assembler is called like this:
> >
> >/tmp/sh/b-gcc-powerpc-rtems6/./gcc/as -I 
> >/home/EB/sebastian_h/src/gcc/libgcc/../newlib/libc/sys/rtems/include -I 
> >. -I . -I ../../../.././gcc -I /home/EB/sebastian_h/src/gcc/libgcc -I 
> >/home/EB/sebastian_h/src/gcc/libgcc/. -I 
> >/home/EB/sebastian_h/src/gcc/libgcc/../gcc -I 
> >/home/EB/sebastian_h/src/gcc/libgcc/../include -a32 -me500 -mbig -o 
> >_negvdi2.o _negvdi2.s
> >
> >Using -me500 seems to be all right, however, the file contains a machine 
> >directive:
> >
> >     .file    "libgcc2.c"
> >     .machine ppc
> >     .section    ".text"
> >
> >If I remove the ".machine ppc" by hand, the file can be assembled with 
> >the above command line.
> >
> >The affect of the patch is:
> >
> >diff -u _negvdi2.s.before _negvdi2.s.after
> >--- _negvdi2.s.before   2022-01-11 09:07:43.313828636 +0100
> >+++ _negvdi2.s.after    2022-01-11 08:54:08.424946502 +0100
> >@@ -1,5 +1,5 @@
> >         .file   "libgcc2.c"
> >-       .machine power9
> >+       .machine ppc

Which is "correct".  rs6000_machine_from_flags should be updated to know
about these CPUs: if you have a -mcpu= that outputs instructions that
are not .machine ppc (or ppc64), the compiler should emit
some appropriate .machine instead.

> I can try to fix this, however, for me it is not really clear in which 
> direction this should be fixed. I can build the GCC 10.3.0 release (it 
> uses .machine power9). The GCC 10 branch is broken (it uses .machine 
> ppc). Using the .machine directive and a command line option (-me500) 
> seems to be a bit inconsistent. It should be one or the other.

Exactly.

> Would a patch which changes .machine ppc to .machine e500 for -mcpu=8540 
> be the right way to fix this issue?

Yes :-)

Do you have the info needed to make a patch like this for all FSL CPUs?


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 48b8efd732b2..844fee88cf3c 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5765,7 +5765,7 @@  rs6000_machine_from_flags (void)
   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);
+  flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ISEL);
 
   if ((flags & (ISA_3_1_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0)
     return "power10";