[rs6000] Fix PR56010 and PR83743, -mcpu=native use wrong names

The following patch fixes both PR56010 and PR83743.  PR56010 is fixed by
adding an extra altname field to the RS6000_CPU table which matches the
cases where the Linux kernel's AT_PLATFORM name differs from the name GCC
expects.  If we match on the altname, then we return the canonical name.

PR83743 is fixed by catching the case where we do not recognize at all
the AT_PLATFORM value returned by the kernel.  In that case, we emit an
error message and request the user use an explicit cpu name rather than
using "native".

I have tested this by forcing use of non-existant names and kernel alternate
names and verifying we call cc1/cc1plus with the either the correct canonical
cpu names or we emit an error message and quit.

This has bootstrapped and regtested with no errors.  Ok for mainline?

This patch applies fairly easy to the release branches, do we want this
fix there as well?

Peter

	PR target/56010
	PR target/83743
	* config/rs6000/rs6000-cpus.def (RS6000_CPU table): Add alternate cpu
	names.
	* config/rs6000/driver-rs6000.c: #include "diagnostic.h".
	(struct rs6000_ptt): Define new structure.
	(rs6000_supported_cpu_names): New static variable.
	(elf_platform) <cpu>: Define new static variable and use it.
	Translate kernel AT_PLATFORM name to canonical name if needed.
	Error if platform name is unknown.
	* config/rs6000/rs6000.c: Handle extra field in RS6000_CPU table.
	* config/rs6000/default64.h: Likewise.

Hi!

On Wed, Jan 24, 2018 at 03:49:26PM -0600, Peter Bergner wrote:
> The following patch fixes both PR56010 and PR83743.  PR56010 is fixed by
> adding an extra altname field to the RS6000_CPU table which matches the
> cases where the Linux kernel's AT_PLATFORM name differs from the name GCC
> expects.  If we match on the altname, then we return the canonical name.

Don't add this to rs6000-cpus.def; it belongs to the Linux support only.

You also need to translate multiple AT_PLATFORM names for a single -mcpu=,
not necessarily only two, so you probably want a separate translation table
to do just the translation (in driver-rs6000.c ?)

Did you check this against the kernel's arch/powerpc/kernel/cputable.c ?


Segher

On 1/25/18 2:18 PM, Segher Boessenkool wrote:
> Don't add this to rs6000-cpus.def; it belongs to the Linux support only.

Well, the reason I liked adding it to rs6000-cpus.def is that it keeps
the names together, so only one place to hold all of the info.  If you
prefer a separate table, then I can move it to driver-rs6000.c and it
would only need to hold info for AT_PLATFORMs that don't match GCC's
idea of the cpu name (eg, ppc440 versus 440).


> You also need to translate multiple AT_PLATFORM names for a single -mcpu=,
> not necessarily only two, so you probably want a separate translation table
> to do just the translation (in driver-rs6000.c ?)

Ah, I forgot about ppc440gp, even though it is in my list mentioned in the
link below.  I do think that is the only case though, "440", "ppc440" and
"ppc440gp" all being mapped to "440", but yes, that's not handled.


> Did you check this against the kernel's arch/powerpc/kernel/cputable.c ?

I did.  I posted my finding in the bugzilla:

    https://gcc.gnu.org/PR56010#c5

I'll note that I did not try and disambiguate 440 vs 440fp, etc. by looking
at the HWCAP for PPC_FEATURE_HAS_FPU, like I mentioned in the bugzilla entry
above.  That's due to just because the cpu supports HW FP, we cannot know
what ABI the distro that is running on the cpu is using.  I could very well
be using a soft-float ABI.  Thoughts on that?


Ok, I'll move the table to driver-rs6000.c and I'll resubmit.

Peter

On 1/25/18 3:56 PM, Peter Bergner wrote:
> Ok, I'll move the table to driver-rs6000.c and I'll resubmit.

Ok, here is a separate translation table like you wanted.  I still use the
RS6000_CPU table to hold entire list of canonical cpu names, the new
translation table in driver-rs6000.c only contains cpus whose AT_PLATFORM
names do not match their GCC canonical names.  I also added the pa6t to
970 translation you mentioned in the bugzilla.  If you want me to drop
that, that's easy enough to do.

I realize I also failed to mention in the first submission, that I have
added caching of the elf_plaform() result, so we don't have to mount and
scan /proc/self/auxv multiple times.

Is this better?  I did the same unit testing by forcing unknown names
and names that need translation and I've verified it works.  Bootstrap
and regtesting is still running though.

Peter

	PR target/56010
	PR target/83743
	* config/rs6000/driver-rs6000.c: #include "diagnostic.h".
	(rs6000_supported_cpu_names): New static variable.
	(linux_cpu_translation_table): Likewise.
	(elf_platform) <cpu>: Define new static variable and use it.
	Translate kernel AT_PLATFORM name to canonical name if needed.
	Error if platform name is unknown.

Index: gcc/config/rs6000/driver-rs6000.c
===================================================================
--- gcc/config/rs6000/driver-rs6000.c	(revision 256364)
+++ gcc/config/rs6000/driver-rs6000.c	(working copy)
@@ -23,6 +23,7 @@
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
+#include "diagnostic.h"
 #include <stdlib.h>
 
 #ifdef _AIX
@@ -38,6 +39,44 @@
 # include <sys/sysctl.h>
 #endif
 
+#ifdef __linux__
+/* Canonical GCC cpu name table.  */
+static const char *rs6000_supported_cpu_names[] =
+{
+#define RS6000_CPU(NAME, CPU, FLAGS) NAME,
+#include "rs6000-cpus.def"
+#undef RS6000_CPU
+};
+
+/* This table holds a list of cpus where their Linux AT_PLATFORM name differs
+   from their GCC canonical name.  The first column in a row contains the GCC
+   canonical cpu name and the other columns in that row contain AT_PLATFORM
+   names that should be mapped to the canonical name.  */
+
+static const char *linux_cpu_translation_table[][4] = {
+  { "403", "ppc403", NULL },
+  { "405", "ppc405", NULL },
+  { "440", "ppc440", "ppc440gp", NULL },
+  { "476", "ppc470", NULL },
+  { "601", "ppc601", NULL },
+  { "603", "ppc603", NULL },
+  { "604", "ppc604", NULL },
+  { "7400", "ppc7400", NULL },
+  { "7450", "ppc7450", NULL },
+  { "750", "ppc750", NULL },
+  { "823", "ppc823", NULL },
+  { "8540", "ppc8540", NULL },
+  { "8548", "ppc8548", NULL },
+  { "970", "ppc970", "pa6t", NULL },
+  { "cell", "ppc-cell-be", NULL },
+  { "e500mc", "ppce500mc", NULL },
+  { "e5500", "ppce5500", NULL },
+  { "e6500", "ppce6500", NULL },
+  { "power7", "power7+", NULL },
+  { NULL } /* End of table sentinel.  */
+};
+#endif
+
 const char *host_detect_local_cpu (int argc, const char **argv);
 
 #if GCC_VERSION >= 0
@@ -158,15 +197,19 @@
 
 #ifdef __linux__
 
-/* Returns AT_PLATFORM if present, otherwise generic PowerPC.  */
+/* Returns the canonical AT_PLATFORM if present, otherwise NULL.  */
 
 static const char *
 elf_platform (void)
 {
-  int fd;
+  static const char *cpu = NULL;
 
-  fd = open ("/proc/self/auxv", O_RDONLY);
+  /* Use the cached AT_PLATFORM cpu name if we've already determined it.  */
+  if (cpu != NULL)
+    return cpu;
 
+  int fd = open ("/proc/self/auxv", O_RDONLY);
+
   if (fd != -1)
     {
       char buf[1024];
@@ -179,15 +222,60 @@
       if (n > 0)
 	{
 	  for (av = (ElfW(auxv_t) *) buf; av->a_type != AT_NULL; ++av)
-	    switch (av->a_type)
+	    if (av->a_type == AT_PLATFORM)
 	      {
-	      case AT_PLATFORM:
-		return (const char *) av->a_un.a_val;
-
-	      default:
+		cpu = (const char *) av->a_un.a_val;
 		break;
 	      }
 	}
+
+      /* Verify that CPU is either a valid -mcpu=<cpu> option name, or is a
+	 valid alternative name.  If it is a valid alternative name, then use
+	 the canonical name.  */
+      if (cpu != NULL)
+	{
+	  size_t i, j, len = 0;
+	  char *s, *p;
+
+	  /* Check if AT_PLATFORM is a GCC canonical cpu name.  */
+	  for (i = 0; i < ARRAY_SIZE (rs6000_supported_cpu_names); i++)
+	    {
+	      if (!strcmp (cpu, rs6000_supported_cpu_names[i]))
+		return cpu;
+	      len += strlen (rs6000_supported_cpu_names[i]) + 1;
+	    }
+
+	  /* Check if AT_PLATFORM can be translated to a canonical cpu name.  */
+	  for (i = 0; linux_cpu_translation_table[i][0] != NULL; i++)
+	    {
+	      const char *canonical = linux_cpu_translation_table[i][0];
+	      for (j = 1; linux_cpu_translation_table[i][j] != NULL; j++)
+		if (!strcmp (cpu, linux_cpu_translation_table[i][j]))
+		  {
+		    cpu = canonical;
+		    return cpu;
+		  }
+	    }
+
+	  /* The kernel returned an AT_PLATFORM name we do not support.  */
+	  s = XALLOCAVEC (char, len);
+	  p = s;
+	  for (i = 0; i < ARRAY_SIZE (rs6000_supported_cpu_names); i++)
+	    {
+	      size_t arglen = strlen (rs6000_supported_cpu_names[i]);
+	      memcpy (p, rs6000_supported_cpu_names[i], arglen);
+	      p[arglen] = ' ';
+	      p += arglen + 1;
+	    }
+	  p[-1] = 0;
+
+	  fatal_error (
+	    input_location,
+	    "Unsupported cpu name returned from kernel for -mcpu=native: %s\n"
+	    "Please use an explicit cpu name.  Valid cpu names are: %s",
+	    cpu, s);
+	}
+
     }
   return NULL;
 }

Hi Peter,

On Thu, Jan 25, 2018 at 09:39:39PM -0600, Peter Bergner wrote:
> Ok, here is a separate translation table like you wanted.  I still use the
> RS6000_CPU table to hold entire list of canonical cpu names, the new
> translation table in driver-rs6000.c only contains cpus whose AT_PLATFORM
> names do not match their GCC canonical names.  I also added the pa6t to
> 970 translation you mentioned in the bugzilla.  If you want me to drop
> that, that's easy enough to do.

Yeah dropping it is probably best.

> Is this better?  I did the same unit testing by forcing unknown names
> and names that need translation and I've verified it works.  Bootstrap
> and regtesting is still running though.

Looks much better, thanks.  Some smaller things:

> +#ifdef __linux__
> +/* Canonical GCC cpu name table.  */
> +static const char *rs6000_supported_cpu_names[] =
> +{
> +#define RS6000_CPU(NAME, CPU, FLAGS) NAME,
> +#include "rs6000-cpus.def"
> +#undef RS6000_CPU
> +};

Can't you just use processor_target_table here?  Seems like a waste to
duplicate all that.  (Need to make the table non-static then of course).

>  static const char *
>  elf_platform (void)
>  {
> -  int fd;
> +  static const char *cpu = NULL;

Add a comment here please, static funtion-scope variables are confusing
(but handy, in cases like this).

> +	  /* The kernel returned an AT_PLATFORM name we do not support.  */
> +	  s = XALLOCAVEC (char, len);

I don't think building up the list of names in a string buys you anything?

> +	  fatal_error (
> +	    input_location,
> +	    "Unsupported cpu name returned from kernel for -mcpu=native: %s\n"
> +	    "Please use an explicit cpu name.  Valid cpu names are: %s",
> +	    cpu, s);

"Valid cpu names are:\n" and then a loop printing each name?  Or get
fancy and throw in newlines too so it even looks good.

Thanks,


Segher

On 1/29/18 1:23 PM, Segher Boessenkool wrote:
>> I also added the pa6t to 970 translation you mentioned in the bugzilla.>> If you want me to drop that, that's easy enough to do.
>
> Yeah dropping it is probably best.

Will do.


>> +#ifdef __linux__
>> +/* Canonical GCC cpu name table.  */
>> +static const char *rs6000_supported_cpu_names[] =
>> +{
>> +#define RS6000_CPU(NAME, CPU, FLAGS) NAME,
>> +#include "rs6000-cpus.def"
>> +#undef RS6000_CPU
>> +};
> 
> Can't you just use processor_target_table here?  Seems like a waste to
> duplicate all that.  (Need to make the table non-static then of course).

Changing it to static won't help, because we don't link rs6000.o into
xgcc.  That file is linked into cc1, etc, so we don't have access to it.




>>  static const char *
>>  elf_platform (void)
>>  {
>> -  int fd;
>> +  static const char *cpu = NULL;
> 
> Add a comment here please, static funtion-scope variables are confusing
> (but handy, in cases like this).

Will do.



>> +	  /* The kernel returned an AT_PLATFORM name we do not support.  */
>> +	  s = XALLOCAVEC (char, len);
> 
> I don't think building up the list of names in a string buys you anything?
> 
>> +	  fatal_error (
>> +	    input_location,
>> +	    "Unsupported cpu name returned from kernel for -mcpu=native: %s\n"
>> +	    "Please use an explicit cpu name.  Valid cpu names are: %s",
>> +	    cpu, s);
> 
> "Valid cpu names are:\n" and then a loop printing each name?  Or get
> fancy and throw in newlines too so it even looks good.

Well, this isn't fprintf that I can call multiple times to emit my entire
error message.  Once you call fatal_error(), it never returns, so I copied
the way opts-common.c:cmdline_handle_error() emits its error message which
is to build it up and then emit it altogether like this....however, I now
see I copied GCC 5's version of this code.  Doh! :-)

The current version of that code looks like the following, which I should
copy instead.  Unless you have a different suggestion?

      char *s;

      if (e->unknown_error)
        error_at (loc, e->unknown_error, arg);
      else
        error_at (loc, "unrecognized argument in option %qs", opt);

      auto_vec <const char *> candidates;
      for (i = 0; e->values[i].arg != NULL; i++)
        {
          if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
            continue;
          candidates.safe_push (e->values[i].arg);
        }
      const char *hint = candidates_list_and_hint (arg, s, candidates);
      if (hint)
        inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
                option->opt_text, s, hint);
      else
        inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
      XDELETEVEC (s);


Peter

On 1/29/18 2:33 PM, Peter Bergner wrote:
> The current version of that code looks like the following, which I should
> copy instead.  Unless you have a different suggestion?
> 
>       char *s;
> 
>       if (e->unknown_error)
>         error_at (loc, e->unknown_error, arg);
>       else
>         error_at (loc, "unrecognized argument in option %qs", opt);
> 
>       auto_vec <const char *> candidates;
>       for (i = 0; e->values[i].arg != NULL; i++)
>         {
>           if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
>             continue;
>           candidates.safe_push (e->values[i].arg);
>         }
>       const char *hint = candidates_list_and_hint (arg, s, candidates);
>       if (hint)
>         inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
>                 option->opt_text, s, hint);
>       else
>         inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
>       XDELETEVEC (s);

Actually, all the "macic" of this code happens in candidates_list_and_hint()
and that looks much like the GCC 5 code I showed, plus a call to
find_closest_string() which we don't want, so I think just sticking with
the original code in the last patch is probably best.

Peter

Hi!

On Mon, Jan 29, 2018 at 02:33:50PM -0600, Peter Bergner wrote:
> On 1/29/18 1:23 PM, Segher Boessenkool wrote:
> >> +#ifdef __linux__
> >> +/* Canonical GCC cpu name table.  */
> >> +static const char *rs6000_supported_cpu_names[] =
> >> +{
> >> +#define RS6000_CPU(NAME, CPU, FLAGS) NAME,
> >> +#include "rs6000-cpus.def"
> >> +#undef RS6000_CPU
> >> +};
> > 
> > Can't you just use processor_target_table here?  Seems like a waste to
> > duplicate all that.  (Need to make the table non-static then of course).
> 
> Changing it to static won't help, because we don't link rs6000.o into
> xgcc.  That file is linked into cc1, etc, so we don't have access to it.

Oh ah, tricky.  Fine as is then.

> Well, this isn't fprintf that I can call multiple times to emit my entire
> error message.  Once you call fatal_error(), it never returns, so I copied
> the way opts-common.c:cmdline_handle_error() emits its error message which
> is to build it up and then emit it altogether like this....however, I now
> see I copied GCC 5's version of this code.  Doh! :-)
> 
> The current version of that code looks like the following, which I should
> copy instead.  Unless you have a different suggestion?

Looks better yeah :-)  Probably candidates_list_and_hint does similar
nasty character counting, but we don't have to look at it ;-)


Segher


>       char *s;
> 
>       if (e->unknown_error)
>         error_at (loc, e->unknown_error, arg);
>       else
>         error_at (loc, "unrecognized argument in option %qs", opt);
> 
>       auto_vec <const char *> candidates;
>       for (i = 0; e->values[i].arg != NULL; i++)
>         {
>           if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
>             continue;
>           candidates.safe_push (e->values[i].arg);
>         }
>       const char *hint = candidates_list_and_hint (arg, s, candidates);
>       if (hint)
>         inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
>                 option->opt_text, s, hint);
>       else
>         inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
>       XDELETEVEC (s);

On Mon, Jan 29, 2018 at 02:54:15PM -0600, Peter Bergner wrote:
> On 1/29/18 2:33 PM, Peter Bergner wrote:
> > The current version of that code looks like the following, which I should
> > copy instead.  Unless you have a different suggestion?
> > 
> >       char *s;
> > 
> >       if (e->unknown_error)
> >         error_at (loc, e->unknown_error, arg);
> >       else
> >         error_at (loc, "unrecognized argument in option %qs", opt);
> > 
> >       auto_vec <const char *> candidates;
> >       for (i = 0; e->values[i].arg != NULL; i++)
> >         {
> >           if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
> >             continue;
> >           candidates.safe_push (e->values[i].arg);
> >         }
> >       const char *hint = candidates_list_and_hint (arg, s, candidates);
> >       if (hint)
> >         inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> >                 option->opt_text, s, hint);
> >       else
> >         inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
> >       XDELETEVEC (s);
> 
> Actually, all the "macic" of this code happens in candidates_list_and_hint()
> and that looks much like the GCC 5 code I showed, plus a call to
> find_closest_string() which we don't want, so I think just sticking with
> the original code in the last patch is probably best.

Why don't you want that?  It let's the compiler say "hey silly human who
can hardly type his own name correctly(*), you meant -mcpu-power8 where
you said -mcpu=poewr8".  It's a quite useful feature.  Also amusing in
the cases where it fails spectacularly :-)

Plus, removing support for that would be a regression.

(*) not talking about myself here, oh no.


Segher

On 1/29/18 6:30 PM, Segher Boessenkool wrote:
> On Mon, Jan 29, 2018 at 02:54:15PM -0600, Peter Bergner wrote:
> Why don't you want that?  It let's the compiler say "hey silly human who
> can hardly type his own name correctly(*), you meant -mcpu-power8 where
> you said -mcpu=poewr8".  It's a quite useful feature.  Also amusing in
> the cases where it fails spectacularly :-)>
> Plus, removing support for that would be a regression.

We don't want it, because we're only in this code if the user typed
-mcpu=native (a valid option) and the kernel gave us something we don't
know/understand.  If instead the user typed -mcpu=<unknown cpu>, then they
will go thru the opts-common.c code I showed and get the same error they've
always gotten, so it's not a regression.

For example, the new error when using -mcpu=native gives:

gcc -S -O1 -mcpu=native simple.c
xgcc: fatal error: Unsupported cpu name returned from kernel for -mcpu=native: power9
Please use an explicit cpu name.  Valid cpu names are: 401 403 405 405fp 440 440fp 464 464fp 476 476fp 505 601 602 603 603e 604 604e 620 630 740 7400 7450 750 801 821 823 8540 8548 a2 e300c2 e300c3 e500mc e500mc64 e5500 e6500 860 970 cell ec603e G3 G4 G5 titan power3 power4 power5 power5+ power6 power6x power7 power8 powerpc powerpc64 powerpc64le rs64
compilation terminated.

While -mcpu=<unknown cpu>, you get the old error:

gcc -S -O1 -mcpu=foobar simple.c
xgcc: error: unrecognized argument in option ‘-mcpu=foobar’
xgcc: note: valid arguments to ‘-mcpu=’ are: 401 403 405 405fp 440 440fp 464 464fp 476 476fp 505 601 602 603 603e 604 604e 620 630 740 7400 7450 750 801 821 823 8540 8548 860 970 G3 G4 G5 a2 cell e300c2 e300c3 e500mc e500mc64 e5500 e6500 ec603e native power3 power4 power5 power5+ power6 power6x power7 power8 powerpc powerpc64 powerpc64le rs64 titan


Peter

On 1/29/18 6:15 PM, Segher Boessenkool wrote:
>> The current version of that code looks like the following, which I should
>> copy instead.  Unless you have a different suggestion?
> 
> Looks better yeah :-)  Probably candidates_list_and_hint does similar
> nasty character counting, but we don't have to look at it ;-)

It does, but as I mention in the other reply, I don't think we want this,
since the user used a valid option argument (ie, "native") and we don't
want to give a "hint" as a valid option, since their option was valid.
Like I said in the other reply, I think we do want the code the patch
showed.

Either that, or I could still call candidates_list_and_hint() and just
throw the hint away, since it's meaningless.

Peter

On Mon, Jan 29, 2018 at 08:37:09PM -0600, Peter Bergner wrote:
> On 1/29/18 6:30 PM, Segher Boessenkool wrote:
> > On Mon, Jan 29, 2018 at 02:54:15PM -0600, Peter Bergner wrote:
> > Why don't you want that?  It let's the compiler say "hey silly human who
> > can hardly type his own name correctly(*), you meant -mcpu-power8 where
> > you said -mcpu=poewr8".  It's a quite useful feature.  Also amusing in
> > the cases where it fails spectacularly :-)>
> > Plus, removing support for that would be a regression.
> 
> We don't want it, because we're only in this code if the user typed
> -mcpu=native

Ah, I misread it then.  Sorry.


Segher

On Mon, Jan 29, 2018 at 08:55:35PM -0600, Peter Bergner wrote:
> 
> Either that, or I could still call candidates_list_and_hint() and just
> throw the hint away, since it's meaningless.

It's less code, simpler code, so that would be best I think yes.


Segher

On 1/31/18 11:39 AM, Segher Boessenkool wrote:
> On Mon, Jan 29, 2018 at 08:55:35PM -0600, Peter Bergner wrote:
>>
>> Either that, or I could still call candidates_list_and_hint() and just
>> throw the hint away, since it's meaningless.
> 
> It's less code, simpler code, so that would be best I think yes.

Ok, here's the updated patch that uses candidates_list_and hint()
and throws the hint away because we don't want it.  I also removed
the handling of pa6t like you wanted.

Ok for trunk now?  Do we also want this on the release branches?

Peter


	PR target/56010
	PR target/83743
	* config/rs6000/driver-rs6000.c: #include "diagnostic.h".
	#include "opts.h".
	(rs6000_supported_cpu_names): New static variable.
	(linux_cpu_translation_table): Likewise.
	(elf_platform) <cpu>: Define new static variable and use it.
	Translate kernel AT_PLATFORM name to canonical name if needed.
	Error if platform name is unknown.

Index: gcc/config/rs6000/driver-rs6000.c
===================================================================
--- gcc/config/rs6000/driver-rs6000.c	(revision 256364)
+++ gcc/config/rs6000/driver-rs6000.c	(working copy)
@@ -23,6 +23,8 @@
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
+#include "diagnostic.h"
+#include "opts.h"
 #include <stdlib.h>
 
 #ifdef _AIX
@@ -38,6 +40,44 @@
 # include <sys/sysctl.h>
 #endif
 
+#ifdef __linux__
+/* Canonical GCC cpu name table.  */
+static const char *rs6000_supported_cpu_names[] =
+{
+#define RS6000_CPU(NAME, CPU, FLAGS) NAME,
+#include "rs6000-cpus.def"
+#undef RS6000_CPU
+};
+
+/* This table holds a list of cpus where their Linux AT_PLATFORM name differs
+   from their GCC canonical name.  The first column in a row contains the GCC
+   canonical cpu name and the other columns in that row contain AT_PLATFORM
+   names that should be mapped to the canonical name.  */
+
+static const char *linux_cpu_translation_table[][4] = {
+  { "403", "ppc403", NULL },
+  { "405", "ppc405", NULL },
+  { "440", "ppc440", "ppc440gp", NULL },
+  { "476", "ppc470", NULL },
+  { "601", "ppc601", NULL },
+  { "603", "ppc603", NULL },
+  { "604", "ppc604", NULL },
+  { "7400", "ppc7400", NULL },
+  { "7450", "ppc7450", NULL },
+  { "750", "ppc750", NULL },
+  { "823", "ppc823", NULL },
+  { "8540", "ppc8540", NULL },
+  { "8548", "ppc8548", NULL },
+  { "970", "ppc970", NULL },
+  { "cell", "ppc-cell-be", NULL },
+  { "e500mc", "ppce500mc", NULL },
+  { "e5500", "ppce5500", NULL },
+  { "e6500", "ppce6500", NULL },
+  { "power7", "power7+", NULL },
+  { NULL } /* End of table sentinel.  */
+};
+#endif
+
 const char *host_detect_local_cpu (int argc, const char **argv);
 
 #if GCC_VERSION >= 0
@@ -158,15 +198,20 @@
 
 #ifdef __linux__
 
-/* Returns AT_PLATFORM if present, otherwise generic PowerPC.  */
+/* Returns the canonical AT_PLATFORM if present, otherwise NULL.  */
 
 static const char *
 elf_platform (void)
 {
-  int fd;
+  /* Used to cache the result we determine below.  */
+  static const char *cpu = NULL;
 
-  fd = open ("/proc/self/auxv", O_RDONLY);
+  /* Use the cached AT_PLATFORM cpu name if we've already determined it.  */
+  if (cpu != NULL)
+    return cpu;
 
+  int fd = open ("/proc/self/auxv", O_RDONLY);
+
   if (fd != -1)
     {
       char buf[1024];
@@ -179,15 +224,51 @@
       if (n > 0)
 	{
 	  for (av = (ElfW(auxv_t) *) buf; av->a_type != AT_NULL; ++av)
-	    switch (av->a_type)
+	    if (av->a_type == AT_PLATFORM)
 	      {
-	      case AT_PLATFORM:
-		return (const char *) av->a_un.a_val;
-
-	      default:
+		/* Cache the result.  */
+		cpu = (const char *) av->a_un.a_val;
 		break;
 	      }
 	}
+
+      /* Verify that CPU is either a valid -mcpu=<cpu> option name, or is a
+	 valid alternative name.  If it is a valid alternative name, then use
+	 the canonical name.  */
+      if (cpu != NULL)
+	{
+	  size_t i, j;
+	  char *s;
+
+	  /* Check if AT_PLATFORM is a GCC canonical cpu name.  */
+	  for (i = 0; i < ARRAY_SIZE (rs6000_supported_cpu_names); i++)
+	    if (!strcmp (cpu, rs6000_supported_cpu_names[i]))
+	      return cpu;
+
+	  /* Check if AT_PLATFORM can be translated to a canonical cpu name.  */
+	  for (i = 0; linux_cpu_translation_table[i][0] != NULL; i++)
+	    {
+	      const char *canonical = linux_cpu_translation_table[i][0];
+	      for (j = 1; linux_cpu_translation_table[i][j] != NULL; j++)
+		if (!strcmp (cpu, linux_cpu_translation_table[i][j]))
+		  {
+		    /* Cache the result.  */
+		    cpu = canonical;
+		    return cpu;
+		  }
+	    }
+
+	  /* The kernel returned an AT_PLATFORM name we do not support.  */
+	  auto_vec <const char *> candidates;
+	  for (i = 0; i < ARRAY_SIZE (rs6000_supported_cpu_names); i++)
+	    candidates.safe_push (rs6000_supported_cpu_names[i]);
+	  candidates_list_and_hint (cpu, s, candidates);
+	  fatal_error (
+	    input_location,
+	    "Unsupported cpu name returned from kernel for -mcpu=native: %s\n"
+	    "Please use an explicit cpu name.  Valid cpu names are: %s",
+	    cpu, s);
+	}
     }
   return NULL;
 }

On Wed, Jan 31, 2018 at 12:16:37PM -0600, Peter Bergner wrote:
> On 1/31/18 11:39 AM, Segher Boessenkool wrote:
> > On Mon, Jan 29, 2018 at 08:55:35PM -0600, Peter Bergner wrote:
> >>
> >> Either that, or I could still call candidates_list_and_hint() and just
> >> throw the hint away, since it's meaningless.
> > 
> > It's less code, simpler code, so that would be best I think yes.
> 
> Ok, here's the updated patch that uses candidates_list_and hint()
> and throws the hint away because we don't want it.  I also removed
> the handling of pa6t like you wanted.
> 
> Ok for trunk now?  Do we also want this on the release branches?

Yes, okay for trunk.  Thanks!

I think we also want this for 7 (after a bit of burn in).  I wouldn't
bother with 6 though (the problem has existed since 4.7).


Segher


> 	PR target/56010
> 	PR target/83743
> 	* config/rs6000/driver-rs6000.c: #include "diagnostic.h".
> 	#include "opts.h".
> 	(rs6000_supported_cpu_names): New static variable.
> 	(linux_cpu_translation_table): Likewise.
> 	(elf_platform) <cpu>: Define new static variable and use it.
> 	Translate kernel AT_PLATFORM name to canonical name if needed.
> 	Error if platform name is unknown.

On 2/1/18 8:50 AM, Segher Boessenkool wrote:
> I think we also want this for 7 (after a bit of burn in).  I wouldn't
> bother with 6 though (the problem has existed since 4.7).

Ok, committed.  I'll wait a few days before committing the FSF 7 back port.
Thanks!

Peter

On 2/1/18 1:25 PM, Peter Bergner wrote:
> On 2/1/18 8:50 AM, Segher Boessenkool wrote:
>> I think we also want this for 7 (after a bit of burn in).  I wouldn't
>> bother with 6 though (the problem has existed since 4.7).
> 
> Ok, committed.  I'll wait a few days before committing the FSF 7 back port.

And now committed to the FSF 7 release branch.

Peter