[PATCH] powerc: Fix asm machine directive for some CPUs

[PATCH] powerc: Fix asm machine directive for some CPUs

[Sebastian Huber]

For some CPUs, the assembler machine directive cannot be determined by ISA
flags.

gcc/

	PR 104090/target
	* config/rs6000/rs6000.cc (rs6000_machine_from_flags): Use also
	rtems_cpu.
---
 gcc/config/rs6000/rs6000.cc | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 7a4ef5e6c0a8..d37775ece84d 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -5935,6 +5935,34 @@ const char *rs6000_machine;
 const char *
 rs6000_machine_from_flags (void)
 {
+  /* For some CPUs, the machine cannot be determined by ISA flags.  We have to
+     check them first.  */
+  switch (rs6000_cpu)
+    {
+    case PROCESSOR_PPC8540:
+    case PROCESSOR_PPC8548:
+      return "e500";
+
+    case PROCESSOR_PPCE300C2:
+    case PROCESSOR_PPCE300C3:
+      return "e300";
+
+    case PROCESSOR_PPCE500MC:
+      return "e500mc";
+
+    case PROCESSOR_PPCE500MC64:
+      return "e500mc64";
+
+    case PROCESSOR_PPCE5500:
+      return "e5500";
+
+    case PROCESSOR_PPCE6500:
+      return "e6500";
+
+    default:
+      break;
+    }
+
   HOST_WIDE_INT flags = rs6000_isa_flags;
 
   /* Disable the flags that should never influence the .machine selection.  */
-- 
2.26.2

Re: [PATCH] powerc: Fix asm machine directive for some CPUs

[Segher Boessenkool]

Hi!

On Tue, Jan 18, 2022 at 12:51:39PM +0100, Sebastian Huber wrote:
> For some CPUs, the assembler machine directive cannot be determined by ISA
> flags.

That may be the best we can do here, with the current setup, yes.

> 	PR 104090/target
> 	* config/rs6000/rs6000.cc (rs6000_machine_from_flags): Use also
> 	rtems_cpu.

rs6000_cpu you mean :-)

> +  /* For some CPUs, the machine cannot be determined by ISA flags.  We have to
> +     check them first.  */
> +  switch (rs6000_cpu)
> +    {
> +    case PROCESSOR_PPC8540:
> +    case PROCESSOR_PPC8548:
> +      return "e500";

Ah I could not figure out what core those two are.  But you know it :-)

> +    default:
> +      break;

Please don't do that.  You can do

  default:
    break;
    break;
    /* And just to make sure:  */
    break;
    break;

and it will do exactly the same as not having a default at all.  Not
having such useless code is by far the most readable, so please don't
include a default case at all.

Okay with those changes.  Thanks!


Segher

Re: [PATCH] powerc: Fix asm machine directive for some CPUs

[Sebastian Huber]

On 18/01/2022 22:42, Segher Boessenkool wrote:
>> +    default:
>> +      break;
> Please don't do that.  You can do
> 
>    default:
>      break;
>      break;
>      /* And just to make sure:  */
>      break;
>      break;
> 
> and it will do exactly the same as not having a default at all.  Not
> having such useless code is by far the most readable, so please don't
> include a default case at all.

I removed the default case. I hope this is what you wanted.

> 
> Okay with those changes.  Thanks!

Thanks for having a look at this. I would like to back port this patch 
also to the GCC 10 and 11 branches.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

[committed] rs6000: Fix bootstrap

[Jakub Jelinek]

On Wed, Jan 19, 2022 at 07:54:19AM +0100, Sebastian Huber wrote:
> On 18/01/2022 22:42, Segher Boessenkool wrote:
> > > +    default:
> > > +      break;
> > Please don't do that.  You can do
> > 
> >    default:
> >      break;
> >      break;
> >      /* And just to make sure:  */
> >      break;
> >      break;
> > 
> > and it will do exactly the same as not having a default at all.  Not
> > having such useless code is by far the most readable, so please don't
> > include a default case at all.
> 
> I removed the default case. I hope this is what you wanted.

Unfortunately the removal of default: break; breaks bootstrap:
../../gcc/config/rs6000/rs6000.cc: In function ‘const char* rs6000_machine_from_flags()’:
../../gcc/config/rs6000/rs6000.cc:5940:10: error: enumeration value ‘PROCESSOR_PPC601’ not handled in switch [-Werror=switch]
 5940 |   switch (rs6000_cpu)
      |          ^
../../gcc/config/rs6000/rs6000.cc:5940:10: error: enumeration value ‘PROCESSOR_PPC603’ not handled in switch [-Werror=switch]
...
default: break; is needed to tell the -Wswitch warning that it is intentional
that not all enumerators are handled in the switch.

I've committed following as obvious to unbreak the bootstrap.

2022-01-19  Jakub Jelinek  <jakub@redhat.com>

	* config/rs6000/rs6000.cc (rs6000_machine_from_flags): Add default:.

--- gcc/config/rs6000/rs6000.cc.jj
+++ gcc/config/rs6000/rs6000.cc
@@ -5958,6 +5958,9 @@ rs6000_machine_from_flags (void)
 
     case PROCESSOR_PPCE6500:
       return "e6500";
+
+    default:
+      break;
     }
 
   HOST_WIDE_INT flags = rs6000_isa_flags;


	Jakub

Re: [committed] rs6000: Fix bootstrap

[Segher Boessenkool]

On Wed, Jan 19, 2022 at 12:27:32PM +0100, Jakub Jelinek wrote:
> On Wed, Jan 19, 2022 at 07:54:19AM +0100, Sebastian Huber wrote:
> > On 18/01/2022 22:42, Segher Boessenkool wrote:
> > > > +    default:
> > > > +      break;
> > > Please don't do that.  You can do
> > > 
> > >    default:
> > >      break;
> > >      break;
> > >      /* And just to make sure:  */
> > >      break;
> > >      break;
> > > 
> > > and it will do exactly the same as not having a default at all.  Not
> > > having such useless code is by far the most readable, so please don't
> > > include a default case at all.
> > 
> > I removed the default case. I hope this is what you wanted.

It was.

> Unfortunately the removal of default: break; breaks bootstrap:

&^$()^&#%(^&^!

A questionable warning (switch often is used as a "shorthand" for a
bunch of if statements, like here; quotes because it is actually
*longer* in this case).  And combined with -Werror (the scourge of
sanity) it is much worse: we often make worse code just not to have the
mistaken warnings.

> I've committed following as obvious to unbreak the bootstrap.

Thanks!


Segher

Re: [PATCH] powerc: Fix asm machine directive for some CPUs

[Sebastian Huber]

On 19/01/2022 07:54, Sebastian Huber wrote:
> 
>>
>> Okay with those changes.  Thanks!
> 
> Thanks for having a look at this. I would like to back port this patch 
> also to the GCC 10 and 11 branches.

The default is to ask for back ports after a break. Can I back port the 
patch (with the default: break) to GCC 10 and 11 now?

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

Re: [PATCH] powerc: Fix asm machine directive for some CPUs

[Segher Boessenkool]

On Wed, Feb 02, 2022 at 06:07:38PM +0100, Sebastian Huber wrote:
> On 19/01/2022 07:54, Sebastian Huber wrote:
> >>Okay with those changes.  Thanks!
> >
> >Thanks for having a look at this. I would like to back port this patch 
> >also to the GCC 10 and 11 branches.
> 
> The default is to ask for back ports after a break. Can I back port the 
> patch (with the default: break) to GCC 10 and 11 now?

I have many more changes, but I'll deal with that.  Okay for 11 and 10.
Thanks!


Segher