Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]

[Tobias Burnus <tobias@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 17.06.21 14:27, Jakub Jelinek via Gcc-patches wrote:
> How does this interact with --enable-offload-defaulted ?

Well, it requires all configured offload targets, making the
installation mandatory.

I think that's fine and consistent and is as documented.

> Won't -fopenmp=default=-lm force in all configured targets even when say
> nvptx-none mkoffload/offloading compiler is missing?

Yes – but you could use -foffload=-lm instead if you don't want to have this.

I have to admit that I missed that for -foffload=<target>=<options>,
<target> can be a list of targets.
Thus, '-foffload=default=-lm' probably should be supported.

I think -foffload=disable=-... does not make sense & is now rejected,
likewise for: -foffload=disable,default,disable,nvptx-none=-lm.

For consistency, I now accept -foffload=default=-lm.

And for consistency between
   -foffload=hsa,disable,nvptx-none
and
   -foffload=hsa -foffload=disable -foffload=nvptx-none
the first one no longer disables all offload devices but now
acts like the second one and enables nvptx-none, only.

I hope that everything is now consistent but I find -foffload=
syntax wise overengineered.  :-(

  * * *

Updated version – only lightly tested.  I think it is
consistent like that and the documentation should now be
comprehensive.  (I will have to do some additional testing.)

Further comments and thoughts?

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: offload-v2.diff --]
[-- Type: text/x-patch, Size: 11393 bytes --]

Add 'default' to -foffload=; document that flag [PR67300]

It is long overdue to add documentation for -foffload; while in the past
the need for the user to know the flag was limited, it is now needed
rather often by users.

Additionally, as soon as only some devices need specific flags such
as -latomic, all non-specified offload targets get disabled, unless
explicitly specified. To make that easier, the new flag -foffload=default
has been added. That issue came up at
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570627.html

	PR other/67300

gcc/ChangeLog:

	* doc/invoke.texi (-foffload=): Finally add documentation for it.
	* gcc.c (handle_foffload_option): Support -foffload=default; reject
	-foffload=disable=options.
	* lto-wrapper.c (append_offload_options): Accept -foffload=default=options
	by all targets.

libgomp/ChangeLog:

	* testsuite/libgomp.c-c++-common/reduction-16.c: Use -foffload=default.
	* testsuite/libgomp.c-c++-common/reduction-5.c: Likewise.
	* testsuite/libgomp.c-c++-common/reduction-6.c: Likewise.
	* testsuite/libgomp.c/target-44.c: Likewise.


 gcc/doc/invoke.texi                                | 47 +++++++++++++++++++++-
 gcc/gcc.c                                          | 30 ++++++++++++--
 gcc/lto-wrapper.c                                  |  6 ++-
 .../testsuite/libgomp.c-c++-common/reduction-16.c  |  6 ++-
 .../testsuite/libgomp.c-c++-common/reduction-5.c   |  7 +++-
 .../testsuite/libgomp.c-c++-common/reduction-6.c   |  7 +++-
 libgomp/testsuite/libgomp.c/target-44.c            |  6 ++-
 7 files changed, 98 insertions(+), 11 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index fe812cbd512..a97c8dee12b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -203,7 +203,7 @@ in the following sections.
 -fno-asm  -fno-builtin  -fno-builtin-@var{function}  -fgimple@gol
 -fhosted  -ffreestanding @gol
 -fopenacc  -fopenacc-dim=@var{geom} @gol
--fopenmp  -fopenmp-simd @gol
+-fopenmp  -fopenmp-simd -foffload=@var{arg} @gol
 -fms-extensions  -fplan9-extensions  -fsso-struct=@var{endianness} @gol
 -fallow-single-precision  -fcond-mismatch  -flax-vector-conversions @gol
 -fsigned-bitfields  -fsigned-char @gol
@@ -2639,6 +2639,51 @@ Enable handling of OpenMP's SIMD directives with @code{#pragma omp}
 in C/C++ and @code{!$omp} in Fortran. Other OpenMP directives
 are ignored.
 
+@item -foffload=@var{options}
+@itemx -foffload=@var{target-list=options}
+@itemx -foffload=@var{target-list}
+@opindex foffload
+@cindex Offloading
+@cindex OpenACC
+@cindex OpenMP
+Specifies for which offload/non-host/accelerator devices code should
+be generated when using OpenACC (@option{-fopenacc}) -- or OpenMP
+(@option{-fopenmp}) with target regions.  It additionally permits to
+pass arguments to the offload-target compiler.  Note that code compiled
+for one or more offload-target devices can still be executable when some
+or all offload device are unavailable at runtime, in line with and as
+specified by the OpenACC and OpenMP specifications.
+
+The options specified with @option{-foffload=}@var{options} are passed
+to the all enabled offloading compilers.  This can be used to link
+libraries required for the offloaded code such as @code{-lm},
+@code{-latomic}, or @code{-lgfortran}.  In order to apply an option
+to only one or more specific offload targets,
+@option{-foffload=}@var{target-list=options} can be used.
+
+By default, all configured offloading compilers are enabled. This can
+be overridden: When a @code{target-list} appears, either as
+@option{-foffload=}@var{target-list} or as
+@option{-foffload=}@var{target-list=options}, code is only generated
+for those targets that appear in the comma-separated list.  Besides
+offload-target triplets, @code{disable}, which disables all offload
+targets, and @code{default}, which enables all targets supported by
+the compiler, can be used.  The latter is in particular useful when
+all targets should remain enabled while for some targets specific
+@var{options} have been provided.  @option{-foffload} can be specified
+multiple times; for each enabled target, all @var{options} are effective
+that are specified without a target list or where the target list contains
+either its target triplet or @code{default}.  A target is enabled when
+either no @code{target-list} appears or when by concatenating all
+@var{target-list} and evaluating them in sequence, either its target
+triplet or @code{default} appears and is not followed by @code{disable}.
+
+Note that GCC can be configured to permit configured but non-installed
+offload compilers such that by default only the installed offload
+compilers are enabled.  However, indepent of that configuration, as soon
+as a @code{target-list} has been specified, all such enabled target
+compilers must be installed, which are all in case of @code{default}.
+
 @item -fgnu-tm
 @opindex fgnu-tm
 When the option @option{-fgnu-tm} is specified, the compiler
diff --git a/gcc/gcc.c b/gcc/gcc.c
index af286400a4a..9212a81fb79 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3983,6 +3983,7 @@ static void
 handle_foffload_option (const char *arg)
 {
   const char *c, *cur, *n, *next, *end;
+  bool has_options = false;
   char *target;
 
   /* If option argument starts with '-' then no target is specified and we
@@ -3993,6 +3994,12 @@ handle_foffload_option (const char *arg)
   end = strchr (arg, '=');
   if (end == NULL)
     end = strchr (arg, '\0');
+  else
+    {
+      const char *comma = strchr (arg, ',');
+      if (comma < end)
+	has_options = true;
+    }
   cur = arg;
 
   while (cur < end)
@@ -4006,13 +4013,27 @@ handle_foffload_option (const char *arg)
       memcpy (target, cur, next - cur);
       target[next - cur] = '\0';
 
-      /* If 'disable' is passed to the option, stop parsing the option and clean
-         the list of offload targets.  */
+      /* If 'disable' is passed to the option, clean the list of
+	 offload targets.  */
       if (strcmp (target, "disable") == 0)
 	{
+	  if (has_options)
+	    {
+	      error ("options specified for %<-foffload=disable%>");
+	      has_options = false;
+	    }
 	  free (offload_targets);
 	  offload_targets = xstrdup ("");
-	  break;
+	  goto next_item;
+	}
+
+      /* If 'default', replace the list of offloaded targets by all configured
+	 targets; note that this implies offload_targets_default = false.  */
+      if (strcmp (target, "default") == 0)
+	{
+	  free (offload_targets);
+	  offload_targets = xstrdup (OFFLOAD_TARGETS);
+	  goto next_item;
 	}
 
       /* Check that GCC is configured to support the offload target.  */
@@ -4031,7 +4052,7 @@ handle_foffload_option (const char *arg)
 
       if (!c)
 	fatal_error (input_location,
-		     "GCC is not configured to support %s as offload target",
+		     "GCC is not configured to support %qs as offload target",
 		     target);
 
       if (!offload_targets)
@@ -4068,6 +4089,7 @@ handle_foffload_option (const char *arg)
 	    }
 	}
 
+next_item:
       cur = next + 1;
       XDELETEVEC (target);
     }
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 1c2643984f9..c7faaf147b8 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -858,9 +858,11 @@ append_offload_options (obstack *argv_obstack, const char *target,
 		next = opts;
 	      next = (next > opts) ? opts : next;
 
-	      /* Are we looking for this offload target?  */
+	      /* Are we looking for this offload target?
+		 'default' in -foffload=default=-... matches all targets.  */
 	      if (strlen (target) == (size_t) (next - cur)
-		  && strncmp (target, cur, next - cur) == 0)
+		  && (strncmp (target, cur, next - cur) == 0
+		      || strncmp ("default", cur, next-cur) == 0))
 		break;
 
 	      /* Skip the comma or equal sign.  */
diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
index 0eea73b144b..669f5d5b1af 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
@@ -1,5 +1,9 @@
 /* { dg-do run } */
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
+/* { dg-additional-options "-foffload=default -foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
+/* nvptx often needs -latomic (like for this testcase), which is not linked
+   automatically.  However, when the compiler supports the code generation for
+   multiple offload targets including nvptx, the -foffload=default is needed as
+   otherwise the code generation for all other no-host devices is disabled. */
 
 #include <stdlib.h>
 
diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
index 31fa2670312..95fc1b79eed 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
@@ -1,4 +1,9 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload=default -foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* nvptx often needs -latomic (like for this testcase), which is not linked
+   automatically.  However, when the compiler supports the code generation for
+   multiple offload targets including nvptx, the -foffload=default is needed as
+   otherwise the code generation for all other no-host devices is disabled. */
+
 /* C / C++'s logical AND and OR operators take any scalar argument
    which compares (un)equal to 0 - the result 1 or 0 and of type int.
 
diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
index 727e11e4edf..ba2c9bdc2a8 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
@@ -1,4 +1,9 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload=default -foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* nvptx often needs -latomic (like for this testcase), which is not linked
+   automatically.  However, when the compiler supports the code generation for
+   multiple offload targets including nvptx, the -foffload=default is needed as
+   otherwise the code generation for all other no-host devices is disabled. */
+
 /* C / C++'s logical AND and OR operators take any scalar argument
    which compares (un)equal to 0 - the result 1 or 0 and of type int.
 
diff --git a/libgomp/testsuite/libgomp.c/target-44.c b/libgomp/testsuite/libgomp.c/target-44.c
index b95e807a114..9659fe3ee46 100644
--- a/libgomp/testsuite/libgomp.c/target-44.c
+++ b/libgomp/testsuite/libgomp.c/target-44.c
@@ -1,4 +1,8 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload=default -foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* nvptx often needs -latomic (like for this testcase), which is not linked
+   automatically.  However, when the compiler supports the code generation for
+   multiple offload targets including nvptx, the -foffload=default is needed as
+   otherwise the code generation for all other no-host devices is disabled. */
 
 #include <stdlib.h>