diff mbox series

GCN: Fix --with-arch= handling in mkoffload [PR111966]

Message ID 5b635551-a171-4cc2-bf5e-a71070740b14@baylibre.com
State New
Headers show
Series GCN: Fix --with-arch= handling in mkoffload [PR111966] | expand

Commit Message

Tobias Burnus April 3, 2024, 9:05 a.m. UTC 
This patch handles --with-arch= in GCN's mkoffload.cc

While mkoffload mostly does not know this and passes it through to the GCN lto1 compiler,
it writes an .o file with debug information - and here the -march= in the ELF flags must
agree with the one in the other files. Hence, it uses now the --with-arch= config argument.

Doing so, there is now a diagnostic if the -march= or --with-arch= is unknown. While the
latter should be rejected at GCC compile time, the latter was not diagnosed in mkoffload
but only later in GCN's compiler.

But as there is now a fatal_error in mkoffload, which comes before the GCN-compiler call,
the 'note:' which devices are available were lost. This has been reinstated by using
the multilib settings. (That's not identical to the compiler supported flags the output
is reasonable, arguable better or worse than lto1.)

Advantage: The output is less cluttered than a later fail.

To make mkoffload errors - and especially this one - more useful, it now also initializes
the colorization / bold.

OK for mainline?

* * *

Example error:

gcn mkoffload: error: unrecognized argument in option '-march=gfx1111'
gcn mkoffload: note: valid arguments to '-march=' are: gfx906, gfx908, gfx90a, gfx1030, gfx1036, gfx1100, gfx1103

where on my TERM=xterm-256color,  'gcn mkoffload:' and the quoted texts are in bold,
'error:' is red and 'note:' is cyan.

Compared to cc1, the 'note:' lacks 'fiji', the list is separated by ', '
instead of ' ', and cc1 has a "; did you mean 'gfx1100'?".
And the program name is 'gcn mkoffload' instead of 'cc1'.

Tobias

PS: The generated multilib list could be later changed to be based on the gcn-.def file;
or we just keep the multiconfig variant of this patch.

Comments

Andrew Stubbs April 3, 2024, 10:20 a.m. UTC | #1 
On 03/04/2024 10:05, Tobias Burnus wrote:
> This patch handles --with-arch= in GCN's mkoffload.cc
> 
> While mkoffload mostly does not know this and passes it through to the 
> GCN lto1 compiler,
> it writes an .o file with debug information - and here the -march= in 
> the ELF flags must
> agree with the one in the other files. Hence, it uses now the 
> --with-arch= config argument.
> 
> Doing so, there is now a diagnostic if the -march= or --with-arch= is 
> unknown. While the
> latter should be rejected at GCC compile time, the latter was not 
> diagnosed in mkoffload
> but only later in GCN's compiler.
> 
> But as there is now a fatal_error in mkoffload, which comes before the 
> GCN-compiler call,
> the 'note:' which devices are available were lost. This has been 
> reinstated by using
> the multilib settings. (That's not identical to the compiler supported 
> flags the output
> is reasonable, arguable better or worse than lto1.)
> 
> Advantage: The output is less cluttered than a later fail.
> 
> To make mkoffload errors - and especially this one - more useful, it now 
> also initializes
> the colorization / bold.
> 
> OK for mainline?

OK. Thanks for fixing this.

Andrew

> 
> * * *
> 
> Example error:
> 
> gcn mkoffload: error: unrecognized argument in option '-march=gfx1111'
> gcn mkoffload: note: valid arguments to '-march=' are: gfx906, gfx908, 
> gfx90a, gfx1030, gfx1036, gfx1100, gfx1103
> 
> where on my TERM=xterm-256color,  'gcn mkoffload:' and the quoted texts 
> are in bold,
> 'error:' is red and 'note:' is cyan.
> 
> Compared to cc1, the 'note:' lacks 'fiji', the list is separated by ', '
> instead of ' ', and cc1 has a "; did you mean 'gfx1100'?".
> And the program name is 'gcn mkoffload' instead of 'cc1'.
> 
> Tobias
> 
> PS: The generated multilib list could be later changed to be based on 
> the gcn-.def file;
> or we just keep the multiconfig variant of this patch.

I think a .def file would be more future-proof if we ever have multilibs 
for options other than -march, but this works for now.

Andrew
diff mbox series

Patch

GCN: Fix --with-arch= handling in mkoffload [PR111966]

The default -march= setting used in mkoffload did not reflect the modified
default set by GCC's configure-time --with-arch=, causing issues when
generating debug code.

gcc/ChangeLog:

	PR other/111966
	* config/gcn/mkoffload.cc (get_arch): New; moved -march= flag
	handling from ...
	(main): ... here; call it to handle --with-arch config option
	and -march= commandline.

 gcc/config/gcn/mkoffload.cc | 90 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 18 deletions(-)

diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index 04356b86195..31266d2099b 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -35,6 +35,8 @@ 
 #include "gomp-constants.h"
 #include "simple-object.h"
 #include "elf.h"
+#include "configargs.h"  /* For configure_default_options.  */
+#include "multilib.h"  /* For multilib_options.  */
 
 /* These probably won't (all) be in elf.h for a while.  */
 #undef  EM_AMDGPU
@@ -846,6 +848,62 @@  compile_native (const char *infile, const char *outfile, const char *compiler,
   obstack_free (&argv_obstack, NULL);
 }
 
+int
+get_arch (const char *str, const char *with_arch_str)
+{
+  if (strcmp (str, "fiji") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX803;
+  else if (strcmp (str, "gfx900") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX900;
+  else if (strcmp (str, "gfx906") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX906;
+  else if (strcmp (str, "gfx908") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX908;
+  else if (strcmp (str, "gfx90a") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX90a;
+  else if (strcmp (str, "gfx1030") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX1030;
+  else if (strcmp (str, "gfx1036") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX1036;
+  else if (strcmp (str, "gfx1100") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX1100;
+  else if (strcmp (str, "gfx1103") == 0)
+    return EF_AMDGPU_MACH_AMDGCN_GFX1103;
+
+  error ("unrecognized argument in option %<-march=%s%>", str);
+
+  /* The suggestions are based on the configured multilib support; the compiler
+     itself might support more.  */
+  if (multilib_options[0] != '\0')
+    {
+      /* Example: "march=gfx900/march=gfx906" */
+      char *args = (char *) alloca (strlen (multilib_options));
+      const char *p = multilib_options, *q = NULL;
+      args[0] = '\0';
+      while (true)
+	{
+	  p = strchr (p, '=');
+	  if (!p)
+	    break;
+	  if (q)
+	    strcat (args, ", ");
+	  ++p;
+	  q = strchr (p, '/');
+	  if (q)
+	    strncat (args, p, q-p);
+	  else
+	    strcat (args, p);
+	}
+      inform (UNKNOWN_LOCATION, "valid arguments to %<-march=%> are: %s", args);
+    }
+  else if (with_arch_str)
+    inform (UNKNOWN_LOCATION, "valid argument to %<-march=%> is %qs", with_arch_str);
+
+  exit (FATAL_EXIT_CODE);
+
+  return 0;
+}
+
 int
 main (int argc, char **argv)
 {
@@ -853,9 +911,21 @@  main (int argc, char **argv)
   FILE *out = stdout;
   FILE *cfile = stdout;
   const char *outname = 0;
+  const char *with_arch_str = NULL;
 
   progname = tool_name;
+  gcc_init_libintl ();
   diagnostic_initialize (global_dc, 0);
+  diagnostic_color_init (global_dc);
+
+  for (size_t i = 0; i < ARRAY_SIZE (configure_default_options); i++)
+    if (configure_default_options[i].name != NULL
+	&& strcmp (configure_default_options[i].name, "arch") == 0)
+      {
+	with_arch_str = configure_default_options[0].value;
+	elf_arch = get_arch (configure_default_options[0].value, NULL);
+	break;
+      }
 
   obstack_init (&files_to_cleanup);
   if (atexit (mkoffload_cleanup) != 0)
@@ -961,24 +1031,8 @@  main (int argc, char **argv)
       else if (strcmp (argv[i], "-dumpbase") == 0
 	       && i + 1 < argc)
 	dumppfx = argv[++i];
-      else if (strcmp (argv[i], "-march=fiji") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX803;
-      else if (strcmp (argv[i], "-march=gfx900") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX900;
-      else if (strcmp (argv[i], "-march=gfx906") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX906;
-      else if (strcmp (argv[i], "-march=gfx908") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX908;
-      else if (strcmp (argv[i], "-march=gfx90a") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX90a;
-      else if (strcmp (argv[i], "-march=gfx1030") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX1030;
-      else if (strcmp (argv[i], "-march=gfx1036") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX1036;
-      else if (strcmp (argv[i], "-march=gfx1100") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX1100;
-      else if (strcmp (argv[i], "-march=gfx1103") == 0)
-	elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX1103;
+      else if (startswith (argv[i], "-march="))
+	elf_arch = get_arch (argv[i] + strlen ("-march="), with_arch_str);
 #define STR "-mstack-size="
       else if (startswith (argv[i], STR))
 	gcn_stack_size = atoi (argv[i] + strlen (STR));