public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] Add 'default' to -foffload=; document that flag [PR67300]
@ 2021-06-17 12:08 Tobias Burnus
  2021-06-17 12:27 ` Jakub Jelinek
  0 siblings, 1 reply; 19+ messages in thread
From: Tobias Burnus @ 2021-06-17 12:08 UTC (permalink / raw)
  To: Thomas Schwinge, Richard Biener, Jakub Jelinek, Sandra Loosemore,
	gcc-patches

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

Hi all, this patch does two things:

(A) Documentation for -fopenmp

This finally adds documentation - which was lacking a long time but
is increasingly demanded. Often used options:
  -foffload='-lm' and '-lgfortran'
  -foffload=(nvptx-none=)-latomic
  -foffload=(amdgcn-amdhsa=)-march=gfx906

  * * *

(B) -foffload=default

Came up for the testsuite as libatomic is sometimes needed for nvptx
and unsupported (not build) for GCN.
When GCC is configured to support more than one offload target:
* -foffload=-latomic →  fails to build on GCN
* -foffload=nvptx-none=-latomic →  disables all but nvptx

However, it might be also useful in the real world.

  * * *

Loosely tested without offloading support and with only one offload target.
As the following works, I assume the patch is fine – but I would not mind
if someone else would test it, who built GCC with multiple offload targets.

For testing, I want to note that
   gcc -v 2>&1|grep OFFLOAD_TARGET
shows the configured targets (and OFFLOAD_TARGET_DEFAULT=1 when
configured with --enable-offload-defaulted).

As soon as -foffload=... has been specified, the output is changed to
only include the specified devices.

  * * *

Thoughts? Comments? Suggestions? Test results? OK?

  * * *

Crossref:

* Configure option --enable-offload-defaulted,
   see https://gcc.gnu.org/install/configure.html
* PR about documenting -foffload: https://gcc.gnu.org/PR67300
* Current documentation in the wiki: https://gcc.gnu.org/wiki/Offloading
   (First line: 'compilation options' link)
* Email thread about -foffload=default, -foffload=nvptx-none=-latomic
   disabling other hosts and (next in thread) automatically linking
   -latomic.
   https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570627.html

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.diff --]
[-- Type: text/x-patch, Size: 8394 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.

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                                | 40 +++++++++++++++++++++-
 gcc/gcc.c                                          |  7 ++++
 .../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 +++-
 6 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index fe812cbd512..ab2ac0e6b3b 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,44 @@ 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{offload-target-triplet}
+@itemx -foffload=@var{offload-target-triplet}=@var{flags}
+@itemx -foffload=@var{flags}
+@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 option @option{-foffload} can be specified multiple times and is
+accumulative, except that @code{-foffload=disable} disables the code
+generation for all offload-target devices that were enabled before that
+option.  By default, code for all supported offload devices is generated;
+however, as soon as any @option{-foffload} with a target triplet has
+been specified, code is only generated for those target triplets
+that appeared in an @option{-foffload} option - that is independent of
+whether only the triplet or triplet plus flags have been used.  By
+specifying @code{-foffload=default} in addition, the code generation
+for all supported devices is enabled.  Note that GCC can be configured
+such that non-installed offload compilers are ignored; in that case,
+using @code{-foffload=default} enforces the compilation with all
+configured devices, which makes the installation of all offload
+compilers mandatory.
+
+The flags specified with @option{-foffload=}@var{flags} are passed to
+the all enabled offloading compilers; whereas using
+@option{-foffload=}@var{offload-target-triplet}=@var{flags}, the flags
+are only passed to the specified compiler.  Typical applications is to
+pass target-specific flags to the offloading compiler or to link
+libraries required for the offloaded code such as @code{-lm},
+@code{-latomic}, or @code{-lgfortran}.
+
 @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..73385b56a89 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4015,6 +4015,13 @@ handle_foffload_option (const char *arg)
 	  break;
 	}
 
+      if (strcmp (target, "default") == 0)
+	{
+	  free (offload_targets);
+	  offload_targets = xstrdup (OFFLOAD_TARGETS);
+	  break;
+	}
+
       /* Check that GCC is configured to support the offload target.  */
       c = OFFLOAD_TARGETS;
       while (c)
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>
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 12:08 [Patch] Add 'default' to -foffload=; document that flag [PR67300] Tobias Burnus
@ 2021-06-17 12:27 ` Jakub Jelinek
  2021-06-17 16:03   ` Tobias Burnus
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Jelinek @ 2021-06-17 12:27 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Thomas Schwinge, Richard Biener, Sandra Loosemore, gcc-patches

On Thu, Jun 17, 2021 at 02:08:07PM +0200, Tobias Burnus wrote:
> +@item -foffload=@var{offload-target-triplet}
> +@itemx -foffload=@var{offload-target-triplet}=@var{flags}
> +@itemx -foffload=@var{flags}
> +@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.

I think default is not offload-target-triplet, so either we should rename
the @var and then say that it is either offload-target-triplet or
@code{default} or @code{disable}, or we should list those default and
disable cases above as separate @itemx.

> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -4015,6 +4015,13 @@ handle_foffload_option (const char *arg)
>  	  break;
>  	}
>  
> +      if (strcmp (target, "default") == 0)
> +	{
> +	  free (offload_targets);
> +	  offload_targets = xstrdup (OFFLOAD_TARGETS);
> +	  break;
> +	}
> +

How does this interact with --enable-offload-defaulted ?  Won't
-fopenmp=default=-lm force in all configured targets even when say
nvptx-none mkoffload/offloading compiler is missing?

And how does it interact with previous -foffload=?

I mean, -foffload=nvptx-none=-latomic -foffload=default=-lm

	Jakub


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 12:27 ` Jakub Jelinek
@ 2021-06-17 16:03   ` Tobias Burnus
  2021-06-17 17:41     ` Sandra Loosemore
  0 siblings, 1 reply; 19+ messages in thread
From: Tobias Burnus @ 2021-06-17 16:03 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: gcc-patches, Sandra Loosemore, Richard Biener, Thomas Schwinge

[-- 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>
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 16:03   ` Tobias Burnus
@ 2021-06-17 17:41     ` Sandra Loosemore
  2021-06-17 17:50       ` Jakub Jelinek
  0 siblings, 1 reply; 19+ messages in thread
From: Sandra Loosemore @ 2021-06-17 17:41 UTC (permalink / raw)
  To: Tobias Burnus, Jakub Jelinek; +Cc: gcc-patches, Richard Biener, Thomas Schwinge

On 6/17/21 10:03 AM, Tobias Burnus wrote:

> 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?

Hmmm, I had started to put together some comments on 
grammar/punctuation/markup on the first version before the second 
iteration showed up in my mailbox, but more critically I could not 
figure out whether -foffload=default is supposed to be exactly identical 
to the default behavior; if it isn't, it should be, or -foffload=default 
ought to be renamed.  So let's get that sorted out first.  I suggest 
reorganizing the documentation to first have a paragraph discussing the 
default behavior, and then move on to how to modify it, with separate 
paragraphs for enabling offload targets explicitly and on adding options 
for offload compilation on all/some targets.

I think it would useful to add an example with a target list and 
multiple options since I think the syntax looks pretty hairy.

-Sandra

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 17:41     ` Sandra Loosemore
@ 2021-06-17 17:50       ` Jakub Jelinek
  2021-06-17 19:28         ` Tobias Burnus
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Jelinek @ 2021-06-17 17:50 UTC (permalink / raw)
  To: Sandra Loosemore
  Cc: Tobias Burnus, Richard Biener, gcc-patches, Thomas Schwinge

On Thu, Jun 17, 2021 at 11:41:39AM -0600, Sandra Loosemore wrote:
> On 6/17/21 10:03 AM, Tobias Burnus wrote:
> 
> > 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?
> 
> Hmmm, I had started to put together some comments on
> grammar/punctuation/markup on the first version before the second iteration
> showed up in my mailbox, but more critically I could not figure out whether
> -foffload=default is supposed to be exactly identical to the default
> behavior; if it isn't, it should be, or -foffload=default ought to be
> renamed.  So let's get that sorted out first.  I suggest reorganizing the

Yeah.  If we want for --enable-offload-default also all configured targets,
we could add another keyword for it (all), but I'm not sure it would be
useful, because whenever it would be different from default it would mean
the linking would fail because one or more offloading targets that were
configured isn't supported (installed).

We need to figure out what it means -foffload=nvptx-none -foffload=default,
if the latter overrides the former (as if it wasn't specified), or if it
adds all the remaining offload targets that are default in addition to it.
And similarly figure out what happens with the optional flags, if they are
gathered from all the -foffload= options that refer to a particular target,
or taken from the last -foffload option that mentions that target, something
else.

	Jakub


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 17:50       ` Jakub Jelinek
@ 2021-06-17 19:28         ` Tobias Burnus
  2021-06-17 19:40           ` Jakub Jelinek
  0 siblings, 1 reply; 19+ messages in thread
From: Tobias Burnus @ 2021-06-17 19:28 UTC (permalink / raw)
  To: Jakub Jelinek, Sandra Loosemore
  Cc: Richard Biener, gcc-patches, Thomas Schwinge

On 17.06.21 19:50, Jakub Jelinek wrote:

> On Thu, Jun 17, 2021 at 11:41:39AM -0600, Sandra Loosemore wrote:
>> I think it would useful to add an example with a target list and
>> multiple options since I think the syntax looks pretty hairy.

I fully concur that -foffload= is a mess trying to achieve too many
things at the same time. [Hence, I kept rephrasing the description
without reaching a good wording.]

In terms of real-world usage, I think something like the following
covers all real-world needs:

-foffload=disable
-foffload=-latomic -foffload=-lm
-foffload="-lgfortran -lm" -foffload=nvptx-none=-latomic
-foffload=-lm -foffload=nvptx-none=-latomic -foffload=amdgcn-amdhsa=-march=gfx906
-foffload=-fdump-tree-all

Possibly with -foffload=default added to item 3 + 4, but cf. below.

>> Hmmm, I had started to put together some comments on
>> grammar/punctuation/markup on the first version before the second iteration
>> showed up in my mailbox, but more critically I could not figure out whether
>> -foffload=default is supposed to be exactly identical to the default
>> behavior; if it isn't, it should be, or -foffload=default ought to be
>> renamed.  So let's get that sorted out first.  I suggest reorganizing the
> Yeah.  If we want for --enable-offload-default also all configured targets,
> we could add another keyword for it (all), but I'm not sure it would be
> useful, because whenever it would be different from default it would mean
> the linking would fail because one or more offloading targets that were
> configured isn't supported (installed).

I am not sure whether I fully agree with this or not. However:

Let's propose something radical, which probably won't break any real-world
code, avoids the need to add a new -foffload=<something> keyword and is
also intuitive to the user:

* -foffload=<target triplet list>=-option

Suggestion: This no longer affects the list of enabled targets. As by default
all targets are enabled, this one will (kept) be(en) enabled (but might
silently fail if the target lto1 is not installed).

* -foffload=disable  and -foffload=<triplet-list>

This is the only way to modify the list of supported offload devices to those
specified. By adding a triplet explicitly, it will give an error via lto1.

That will solve all issues, possibly except for
   -foffload=-lm -foffload=nxpt-none=-latomic -foffload=amdgcn-amdhsa
some might find it surprising that nvptx offloading will be disabled,
but others might find it natural.


Hence: Do you think this change makes sense? It looks somewhat consistent,
avoids a new -foffload=<something> flag etc.?
It also would solve the testsuite issue without needing to add
a new flag and a comment explaining why the flag is needed.


What do you think? It breaks backward compatibility, but I am not
sure anyone way relying on the current behavior.

   * * *

Compared to the current version, I would also give an error if
  -foffload=<list>
contains 'disable' and any other target entry (including 'disable')
and for '-foffload=disable=<options> – as both does not make any sense.

This does not really change the semantic but avoids odd code.


-foffload=<triplet list> will then restrict it to certain
targets & by specifying them explicitly, lto1 will still give
an error when not available.

> And similarly figure out what happens with the optional flags, if they are
> gathered from all the -foffload= options that refer to a particular target,
> or taken from the last -foffload option that mentions that target, something
> else.

Currently,
   -foffload=-lm -foffload=nvptx-none=-latomic -foffload=-lgfortran
works, taking the arguments "-lm -lgfortran" for all but nvptx-none
and "-lm -latomic -lgfortran" for nvptx-none. I think this really needs
to continue to work.

Tobias

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 19:28         ` Tobias Burnus
@ 2021-06-17 19:40           ` Jakub Jelinek
  2021-06-17 21:57             ` Sandra Loosemore
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Jelinek @ 2021-06-17 19:40 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Sandra Loosemore, Richard Biener, gcc-patches, Thomas Schwinge

On Thu, Jun 17, 2021 at 09:28:00PM +0200, Tobias Burnus wrote:
> I am not sure whether I fully agree with this or not. However:
> 
> Let's propose something radical, which probably won't break any real-world
> code, avoids the need to add a new -foffload=<something> keyword and is
> also intuitive to the user:
> 
> * -foffload=<target triplet list>=-option
> 
> Suggestion: This no longer affects the list of enabled targets. As by default
> all targets are enabled, this one will (kept) be(en) enabled (but might
> silently fail if the target lto1 is not installed).
> 
> * -foffload=disable  and -foffload=<triplet-list>
> 
> This is the only way to modify the list of supported offload devices to those
> specified. By adding a triplet explicitly, it will give an error via lto1.
> 
> That will solve all issues, possibly except for
>   -foffload=-lm -foffload=nxpt-none=-latomic -foffload=amdgcn-amdhsa
> some might find it surprising that nvptx offloading will be disabled,
> but others might find it natural.

Could we introduce a different option which wouldn't imply enabling that
target:
-foffload-options=<target triplet list>=option
and make
-foffload=<target triplet list>=option
imply (expand in the driver)
-foffload-options=<target triplet list>=option -foffload=<target triplet list>
?
That would be mostly backwards compatible, but would allow users to specify options
separately from the enabled target list.
The <target triplet list> in the above cases couldn't include disable or
default, but the -foffload=<target triplet list> case could, and disable
(either in the list or separately) would simply disable all targets (even
those enabled earlier), while default would reset the list to the default
(basically cancel all previous non-options -foffload= options).
And the -foffload-options= would accumulate in the order given on the
command line.

	Jakub


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 19:40           ` Jakub Jelinek
@ 2021-06-17 21:57             ` Sandra Loosemore
  2021-06-17 23:05               ` Tobias Burnus
  0 siblings, 1 reply; 19+ messages in thread
From: Sandra Loosemore @ 2021-06-17 21:57 UTC (permalink / raw)
  To: Jakub Jelinek, Tobias Burnus; +Cc: Richard Biener, gcc-patches, Thomas Schwinge

On 6/17/21 1:40 PM, Jakub Jelinek wrote:
> On Thu, Jun 17, 2021 at 09:28:00PM +0200, Tobias Burnus wrote:
>> I am not sure whether I fully agree with this or not. However:
>>
>> Let's propose something radical, which probably won't break any real-world
>> code, avoids the need to add a new -foffload=<something> keyword and is
>> also intuitive to the user:
>>
>> * -foffload=<target triplet list>=-option
>>
>> Suggestion: This no longer affects the list of enabled targets. As by default
>> all targets are enabled, this one will (kept) be(en) enabled (but might
>> silently fail if the target lto1 is not installed).
>>
>> * -foffload=disable  and -foffload=<triplet-list>
>>
>> This is the only way to modify the list of supported offload devices to those
>> specified. By adding a triplet explicitly, it will give an error via lto1.
>>
>> That will solve all issues, possibly except for
>>    -foffload=-lm -foffload=nxpt-none=-latomic -foffload=amdgcn-amdhsa
>> some might find it surprising that nvptx offloading will be disabled,
>> but others might find it natural.
> 
> Could we introduce a different option which wouldn't imply enabling that
> target:
> -foffload-options=<target triplet list>=option
> and make
> -foffload=<target triplet list>=option
> imply (expand in the driver)
> -foffload-options=<target triplet list>=option -foffload=<target triplet list>
> ?
> That would be mostly backwards compatible, but would allow users to specify options
> separately from the enabled target list.
> The <target triplet list> in the above cases couldn't include disable or
> default, but the -foffload=<target triplet list> case could, and disable
> (either in the list or separately) would simply disable all targets (even
> those enabled earlier), while default would reset the list to the default
> (basically cancel all previous non-options -foffload= options).
> And the -foffload-options= would accumulate in the order given on the
> command line.

I don't feel qualified to comment on the details of the behavior, but 
separating the options and making them more orthogonal to one another 
would certainly make things easier to document.  :-)

One other thing I'd like to see in the docs is how to ask GCC what 
offload targets it is configured to support by default.  This could be 
put in a paragraph that also includes the language about how you need to 
have the compilers for those offload targets installed too.

-Sandra

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 21:57             ` Sandra Loosemore
@ 2021-06-17 23:05               ` Tobias Burnus
  2021-06-18 22:47                 ` Sandra Loosemore
  0 siblings, 1 reply; 19+ messages in thread
From: Tobias Burnus @ 2021-06-17 23:05 UTC (permalink / raw)
  To: Sandra Loosemore, Jakub Jelinek, Tobias Burnus
  Cc: Richard Biener, gcc-patches, Thomas Schwinge

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

On 17.06.21 23:57, Sandra Loosemore wrote:

> On 6/17/21 1:40 PM, Jakub Jelinek wrote:
>> Could we introduce a different option which wouldn't imply enabling that
>> target:
>> -foffload-options=<target triplet list>=option

I think that works – in particular, if we do not document all
the legacy stuff but just how it should be used.

That includes not mentioning that disable and default can
be used in a list and that those can also take arguments.

> I don't feel qualified to comment on the details of the behavior, but
> separating the options and making them more orthogonal to one another
> would certainly make things easier to document.  :-)

I fully concur.

Probably not fully polished, but I have attached a version for discussion.

> One other thing I'd like to see in the docs is how to ask GCC what
> offload targets it is configured to support by default.  This could be
> put in a paragraph that also includes the language about how you need
> to have the compilers for those offload targets installed too.

"gcc -v 2>&1 |grep OFFLOAD_TARGET_NAMES"  shows them.

Note that with the flags under discussion, you can modify
the output, e.g.
* "gcc -v -foffload=disable" removes it
* "gcc -v -foffload=nvptx-none" either gives an error (not supported) or
   just shows that single target.

  * * *

Installing:
Building oneself: "make install" – and everything is there

With a distributions, you need to find the name of the .rpm/.deb
package for the target your are interested in and then run
apt-get / yum / zypper / ...

Hence, when you normally build GCC as a user, for all configured offload
targets, their lto1 should be there - and if lto-wrapper cannot find it,
it aborts with a fatal error.
For distribution compilers, it usually just means that the optional
package is not installed – the missing lto1 should/is then just ignored,
That's the reason why GCC's configure script now has a flag
--enable-offload-defaulted to toggle between supporting optional packages
(distro use) and always errors (normal use).

If you want to know how to build it, have a lot of fun with incomplete
documentation and a look at https://gcc.gnu.org/wiki/Offloading or
at the SUSE/Debian/Red Hat build scripts or g-t-s – and expect that
you won't finish soon ...

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-v3.diff --]
[-- Type: text/x-patch, Size: 12371 bytes --]

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

As -foffload={options,targets,targets=options} is very convoluted,
it has been split into -foffload=targets (supporting the old syntax
for backward compatibilty) and -foffload-options={options,target=options}.

Only the new syntax is documented.

Additionally, -foffload=default is supported, which can reset the
devices after -foffload=disable / -foffload=targets to the default,
if needed.

 gcc/common.opt                                     |  10 ++-
 gcc/doc/invoke.texi                                |  41 +++++++++
 gcc/gcc.c                                          | 100 +++++++++++++++++----
 gcc/lto-opts.c                                     |   3 +-
 gcc/lto-wrapper.c                                  |  10 +--
 gcc/opts.c                                         |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-16.c  |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-5.c   |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-6.c   |   2 +-
 libgomp/testsuite/libgomp.c/target-44.c            |   2 +-
 10 files changed, 140 insertions(+), 34 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index a1353e06bdc..a695a8c5964 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2095,9 +2095,15 @@ fnon-call-exceptions
 Common Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions.
 
+; -foffload=<targets> is documented
+; -foffload=<targets>=<options> is supported for backward compatibility
 foffload=
-Common Driver Joined MissingArgError(options or targets missing after %qs)
--foffload=<targets>=<options>	Specify offloading targets and options for them.
+Driver Joined MissingArgError(targets missing after %qs)
+-foffload=<targets>	Specify offloading targets
+
+foffload-options=
+Common Driver Joined MissingArgError(options or targets=options missing after %qs)
+-foffload=<targets>=<options>	Specify options for the offloading targets
 
 foffload-abi=
 Common Joined RejectNegative Enum(offload_abi) Var(flag_offload_abi) Init(OFFLOAD_ABI_UNSET)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index af2ce189fae..82993fa2c1d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -204,6 +204,7 @@ in the following sections.
 -fhosted  -ffreestanding @gol
 -fopenacc  -fopenacc-dim=@var{geom} @gol
 -fopenmp  -fopenmp-simd @gol
+-foffload=@var{arg} -foffload-options=@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 +2640,46 @@ 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=disable
+@itemx -foffload=default
+@itemx -foffload=@var{target-list}
+@opindex foffload
+@cindex Offloading
+@cindex OpenACC
+@cindex OpenMP
+Specify for which offload targets code should be generated.  By default,
+code for all supported targets is generated.  Using
+@option{-foffload=disable} only code for the host fallback is
+generated, while @option{-foffload=}@var{target-list} generates code
+only for the specified comma-separated list of offload-targets triplets.
+To reset the value to the default, @option{-foffload=default} can be
+used.
+
+Note: Running the compiler with @option{-v} shows the list of
+configured offload targets under @code{OFFLOAD_TARGET_NAMES}.
+
+@item -foffload-options=@var{options}
+@itemx -foffload-options=@var{target-triplet-list}=@var{options}
+@opindex foffload
+@cindex Offloading
+@cindex OpenACC
+@cindex OpenMP
+
+Specify the options passed to the offload-target compiler. While using
+@option{-foffload-options=}@var{options} passes the options to all enabled
+offloading compiler,
+@option{-foffload-options=}@var{target-triplet-list}=@var{options} can
+be used to pass them only to the specified comma-separated list of
+target triplets.
+
+Typical commandlines are
+
+@smallexample
+-foffload=-lgfortran -foffload=-lm
+-foffload=-lm -foffload=nvptx-none=-latomic
+-foffload=amdgcn-amdhsa=-march=gfx906 -foffload=nvptx-none="-latomic -lm"
+@end smallexample
+
 @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..203e6e5f8ed 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3977,6 +3977,68 @@ driver_wrong_lang_callback (const struct cl_decoded_option *decoded,
 static const char *spec_lang = 0;
 static int last_language_n_infiles;
 
+
+/* Check that GCC is configured to support the offload target.  */
+
+static void
+check_offload_target_name (const char *target, ptrdiff_t len)
+{
+  const char *n, *c = OFFLOAD_TARGETS;
+  char *target2 = NULL;
+  while (c)
+    {
+      n = strchr (c, ',');
+      if (n == NULL)
+	n = strchr (c, '\0');
+      if (len == n - c && strncmp (target, c, n - c) == 0)
+	break;
+      c = *n ? n + 1 : NULL;
+    }
+  if (!c)
+    {
+      if (target[len] != '\0')
+	{
+	  target2 = XNEWVEC (char, len + 1);
+	  memcpy (target2, target, len);
+	  target2[len] = '\0';
+	}
+      fatal_error (input_location,
+		 "GCC is not configured to support %qs as offload target",
+		 target2 ? target2 : target);
+      XDELETEVEC (target2);
+    }
+}
+
+/* Sanity check for -foffload-options.  */
+
+static void
+check_foffload_target_names (const char *arg)
+{
+  const char *cur, *next, *end;
+  /* If option argument starts with '-' then no target is specified and we
+     do not need to parse it.  */
+  if (arg[0] == '-')
+    return;
+  end = strchr (arg, '=');
+  if (end == NULL)
+    {
+      error ("%<=options%> missing after %<-foffload-options=target%>");
+      return;
+    }
+
+  cur = arg;
+  while (cur < end)
+    {
+      next = strchr (cur, ',');
+      if (next == NULL)
+	next = end;
+      next = (next > end) ? end : next;
+
+      check_offload_target_name (cur, next - cur);
+      cur = next + 1;
+   }
+}
+
 /* Parse -foffload option argument.  */
 
 static void
@@ -4006,33 +4068,25 @@ 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 and return, even if more targets follow. */
       if (strcmp (target, "disable") == 0)
 	{
 	  free (offload_targets);
 	  offload_targets = xstrdup ("");
-	  break;
+	  return;
 	}
 
-      /* Check that GCC is configured to support the offload target.  */
-      c = OFFLOAD_TARGETS;
-      while (c)
+      /* Reset offloading list and continue.  */
+      if (strcmp (target, "default") == 0)
 	{
-	  n = strchr (c, ',');
-	  if (n == NULL)
-	    n = strchr (c, '\0');
-
-	  if (next - cur == n - c && strncmp (target, c, n - c) == 0)
-	    break;
-
-	  c = *n ? n + 1 : NULL;
+	  free (offload_targets);
+	  offload_targets = NULL;
+	  goto next_item;
 	}
 
-      if (!c)
-	fatal_error (input_location,
-		     "GCC is not configured to support %s as offload target",
-		     target);
+      /* Check that GCC is configured to support the offload target.  */
+      check_offload_target_name (target, next - cur);
 
       if (!offload_targets)
 	{
@@ -4067,7 +4121,7 @@ handle_foffload_option (const char *arg)
 	      memcpy (offload_targets + offload_targets_len, target, next - cur + 1);
 	    }
 	}
-
+next_item:
       cur = next + 1;
       XDELETEVEC (target);
     }
@@ -4499,8 +4553,16 @@ driver_handle_option (struct gcc_options *opts,
       flag_wpa = "";
       break;
 
+    case OPT_foffload_options_:
+      check_foffload_target_names (arg);
+      break;
+
     case OPT_foffload_:
       handle_foffload_option (arg);
+      if (arg[0] == '-' || NULL != strchr (arg, '='))
+	save_switch (concat ("-foffload-options=", arg, NULL),
+		     0, NULL, validated, true);
+      do_save = false;
       break;
 
     default:
diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index 6dd55b68072..9496b3c8e0b 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -174,7 +174,8 @@ lto_write_options (void)
 	 We do not need those.  The only exception is -foffload option, if we
 	 write it in offload_lto section.  Also drop all diagnostic options.  */
       if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
-	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
+	  && (!lto_stream_offload_p
+	      || option->opt_index != OPT_foffload_options_))
 	continue;
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 1c2643984f9..aae48aff100 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -453,7 +453,7 @@ merge_and_complain (vec<cl_decoded_option> decoded_options,
 	  break;
 
 
-	case OPT_foffload_:
+	case OPT_foffload_options_:
 	  decoded_options.safe_push (*foption);
 	  break;
 
@@ -833,7 +833,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       unsigned argc;
       cl_decoded_option *option = &options[i];
 
-      if (option->opt_index != OPT_foffload_)
+      if (option->opt_index != OPT_foffload_options_)
 	continue;
 
       /* If option argument starts with '-' then no target is specified.  That
@@ -844,11 +844,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       else
 	{
 	  opts = strchr (option->arg, '=');
-	  /* If there are offload targets specified, but no actual options,
-	     there is nothing to do here.  */
-	  if (!opts)
-	    continue;
-
+	  gcc_assert (opts);
 	  cur = option->arg;
 
 	  while (cur < opts)
diff --git a/gcc/opts.c b/gcc/opts.c
index 52e9e3a9df9..3a43dd43398 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2706,7 +2706,7 @@ common_handle_option (struct gcc_options *opts,
       /* Deferred.  */
       break;
 
-    case OPT_foffload_:
+    case OPT_foffload_options_:
       /* Deferred.  */
       break;
 
diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
index 0eea73b144b..4bf62c3828f 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target offload_target_nvptx } } */
 
 #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..52f23e35b9a 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 /* 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..62e81506bdd 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 /* 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..a5da81d7e23 100644
--- a/libgomp/testsuite/libgomp.c/target-44.c
+++ b/libgomp/testsuite/libgomp.c/target-44.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 
 #include <stdlib.h>
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-17 23:05               ` Tobias Burnus
@ 2021-06-18 22:47                 ` Sandra Loosemore
  2021-06-28 11:28                   ` Tobias Burnus
  0 siblings, 1 reply; 19+ messages in thread
From: Sandra Loosemore @ 2021-06-18 22:47 UTC (permalink / raw)
  To: Tobias Burnus, Jakub Jelinek, Tobias Burnus
  Cc: Richard Biener, gcc-patches, Thomas Schwinge

On 6/17/21 5:05 PM, Tobias Burnus wrote:
> On 17.06.21 23:57, Sandra Loosemore wrote:
> 
>> On 6/17/21 1:40 PM, Jakub Jelinek wrote:
>>> Could we introduce a different option which wouldn't imply enabling that
>>> target:
>>> -foffload-options=<target triplet list>=option
> 
> I think that works – in particular, if we do not document all
> the legacy stuff but just how it should be used.
> 
> That includes not mentioning that disable and default can
> be used in a list and that those can also take arguments.
> 
>> I don't feel qualified to comment on the details of the behavior, but 
>> separating the options and making them more orthogonal to one another 
>> would certainly make things easier to document.  :-)
> 
> I fully concur.
> 
> Probably not fully polished, but I have attached a version for discussion.

Thanks.  The description of the options is a lot easier to follow now, 
so I mostly have only nit-picky Texinfo/grammar/terminology comments 
about the docs now.

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index af2ce189fae..82993fa2c1d 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -204,6 +204,7 @@ in the following sections.
>  -fhosted  -ffreestanding @gol
>  -fopenacc  -fopenacc-dim=@var{geom} @gol
>  -fopenmp  -fopenmp-simd @gol
> +-foffload=@var{arg} -foffload-options=@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

Two spaces instead of one to separate options on the same line in a 
@gccoptlist environment.

The -f options are alphabetized in most of the other @gccoptlist tables 
in the option summary section.  I'm not sure why this group isn't, but 
you get extra credit if you fix that, too.

> @@ -2639,6 +2640,46 @@ 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=disable
> +@itemx -foffload=default
> +@itemx -foffload=@var{target-list}
> +@opindex foffload
> +@cindex Offloading
> +@cindex OpenACC
> +@cindex OpenMP

"OpenACC" and "OpenMP" are far too general to be useful for @cindex 
entries.  "Offloading targets", "OpenACC offloading targets", and 
"OpenMP offloading targets" would be more helpful.

> +Specify for which offload targets code should be generated.  By default,
> +code for all supported targets is generated.  Using
> +@option{-foffload=disable} only code for the host fallback is
> +generated, while @option{-foffload=}@var{target-list} generates code
> +only for the specified comma-separated list of offload-targets triplets.
> +To reset the value to the default, @option{-foffload=default} can be
> +used.
> +
> +Note: Running the compiler with @option{-v} shows the list of
> +configured offload targets under @code{OFFLOAD_TARGET_NAMES}.

I'd like to rephrase this a little bit to avoid some awkward sentence 
construction, and also not introduce the term "triplet" without 
explanation in the user documentation (it's only used in the install and 
internals manuals).  So:

Specify for which OpenMP and OpenACC offload targets code should be 
generated.  The default behavior, equivalent to 
@option{-foffload=default}, is to generate code for all supported 
offload targets.  The @option{-foffload=disable} form generates code 
only for the host fallback, while @option{-foffload=@var{target-list}} 
generates code only for the specified comma-separated list of offload 
targets.

Offload targets are specified in GCC's internal target-triplet format. 
You can run the compiler with @option{-v} to show the list of configured 
offload targets under @code{OFFLOAD_TARGET_NAMES}.

> +
> +@item -foffload-options=@var{options}
> +@itemx -foffload-options=@var{target-triplet-list}=@var{options}
> +@opindex foffload
> +@cindex Offloading
> +@cindex OpenACC
> +@cindex OpenMP

Ditto with the overly-general @cindex entries.  I'd list these as 
"Offloading options" etc instead.

Also use @var{target-list} here instead of @var{target-triplet-list} for 
consistency with the previous option and because triplets are not 
otherwise a user-visible thing in GCC.

> +
> +Specify the options passed to the offload-target compiler. While using
> +@option{-foffload-options=}@var{options} passes the options to all enabled
> +offloading compiler,
> +@option{-foffload-options=}@var{target-triplet-list}=@var{options} can
> +be used to pass them only to the specified comma-separated list of
> +target triplets.
> +

Again to fix sentence construction and markup issues:

With @option{-foffload-options=@var{options}}, GCC passes the specified 
@var{options} to the compilers for all enabled offloading targets.  You 
can specify options that apply only to a specific target or targets by 
using the @option{-foffload-options=@var{target-list}=@var{options}}
form.  The @var{target-list} is a comma-separated list in the same 
format as for the @option{-foffload=} option.

> +Typical commandlines are

"command lines", two words.

> +
> +@smallexample
> +-foffload=-lgfortran -foffload=-lm
> +-foffload=-lm -foffload=nvptx-none=-latomic
> +-foffload=amdgcn-amdhsa=-march=gfx906 -foffload=nvptx-none="-latomic -lm"
> +@end smallexample
> +
>  @item -fgnu-tm
>  @opindex fgnu-tm
>  When the option @option{-fgnu-tm} is specified, the compiler

Thanks for working on this!  :-)

-Sandra

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-18 22:47                 ` Sandra Loosemore
@ 2021-06-28 11:28                   ` Tobias Burnus
  2021-06-28 15:51                     ` Tobias Burnus
  0 siblings, 1 reply; 19+ messages in thread
From: Tobias Burnus @ 2021-06-28 11:28 UTC (permalink / raw)
  To: Sandra Loosemore, Jakub Jelinek
  Cc: Richard Biener, gcc-patches, Thomas Schwinge

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

Hi Sandra, hi all,

On 19.06.21 00:47, Sandra Loosemore wrote:
> Thanks. The description of the options is a lot easier to follow now,
> so I mostly have only nit-picky Texinfo/grammar/terminology comments
> about the docs now.
Thanks for your comments/wording suggestions.
> The -f options are alphabetized in most of the other @gccoptlist
> tables in the option summary section.  I'm not sure why this group
> isn't, but you get extra credit if you fix that, too.

Done so. While doing so and then also sorting the list below, I noticed:
* optlist still had -fallow-single-precision but the entry was removed
   in commit f458d1d5d7bd85e412689858ea5d5de681608fbb
* there is -fgnu-tm - but it was missing from the optlist
* I did not fully sort -fsigned-bitfields  -funsigned-bitfields as
   those are in a single entry; hence, I also kept
   -fsigned-char  -funsigned-char together. That also helps when
   reading as they belong together.

>> +@smallexample
>> +-foffload=-lgfortran -foffload=-lm

I did notice that this should now be -foffload-option= ... – now fixed.

Except for the doc/invoke.texi changes unchanged compared to previous
version.

OK?

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-v4.diff --]
[-- Type: text/x-patch, Size: 26117 bytes --]

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

As -foffload={options,targets,targets=options} is very convoluted,
it has been split into -foffload=targets (supporting the old syntax
for backward compatibilty) and -foffload-options={options,target=options}.

Only the new syntax is documented.

Additionally, -foffload=default is supported, which can reset the
devices after -foffload=disable / -foffload=targets to the default,
if needed.

gcc/ChangeLog:

        * common.opt (-foffload=): Update description.
	(-foffload-options=): New.
        * doc/invoke.texi (C Language Options): Sort options
	alphabetical in optlist and also the description itself.
	(-foffload, -foffload-options): New.
        * gcc.c (check_offload_target_name): New, split off from
	handle_foffload_option.
        (check_foffload_target_names): New.
        (handle_foffload_option): Handle -foffload=default.
        (driver_handle_option): Update for -foffload-options.
        * lto-opts.c (lto_write_options): Use -foffload-options
	instead of -foffload.
        * lto-wrapper.c (merge_and_complain, append_offload_options):
	Likewise.
        * opts.c (common_handle_option): Likewise.

diff --git a/gcc/common.opt b/gcc/common.opt
index a1353e06bdc..a695a8c5964 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2095,9 +2095,15 @@ fnon-call-exceptions
 Common Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions.
 
+; -foffload=<targets> is documented
+; -foffload=<targets>=<options> is supported for backward compatibility
 foffload=
-Common Driver Joined MissingArgError(options or targets missing after %qs)
--foffload=<targets>=<options>	Specify offloading targets and options for them.
+Driver Joined MissingArgError(targets missing after %qs)
+-foffload=<targets>	Specify offloading targets
+
+foffload-options=
+Common Driver Joined MissingArgError(options or targets=options missing after %qs)
+-foffload=<targets>=<options>	Specify options for the offloading targets
 
 foffload-abi=
 Common Joined RejectNegative Enum(offload_abi) Var(flag_offload_abi) Init(OFFLOAD_ABI_UNSET)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index af2ce189fae..f8e41d41801 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -197,17 +197,17 @@ in the following sections.
 
 @item C Language Options
 @xref{C Dialect Options,,Options Controlling C Dialect}.
-@gccoptlist{-ansi  -std=@var{standard}  -fgnu89-inline @gol
--fpermitted-flt-eval-methods=@var{standard} @gol
--aux-info @var{filename}  -fallow-parameterless-variadic-functions @gol
--fno-asm  -fno-builtin  -fno-builtin-@var{function}  -fgimple@gol
--fhosted  -ffreestanding @gol
+@gccoptlist{-ansi  -std=@var{standard}  -aux-info @var{filename} @gol
+-fallow-parameterless-variadic-functions  -fno-asm  @gol
+-fno-builtin  -fno-builtin-@var{function}  -fcond-mismatch @gol
+-ffreestanding  -fgimple  -fgnu-tm  -fgnu89-inline  -fhosted @gol
+-flax-vector-conversions  -fms-extensions @gol
 -fopenacc  -fopenacc-dim=@var{geom} @gol
+-foffload=@var{arg} -foffload-options=@var{arg} @gol
 -fopenmp  -fopenmp-simd @gol
--fms-extensions  -fplan9-extensions  -fsso-struct=@var{endianness} @gol
--fallow-single-precision  -fcond-mismatch  -flax-vector-conversions @gol
--fsigned-bitfields  -fsigned-char @gol
--funsigned-bitfields  -funsigned-char}
+-fpermitted-flt-eval-methods=@var{standard} @gol
+-fplan9-extensions -fsigned-bitfields -funsigned-bitfields @gol
+-fsigned-char -funsigned-char -fsso-struct=@var{endianness}}
 
 @item C++ Language Options
 @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
@@ -2448,50 +2448,6 @@ and will almost certainly change in incompatible ways in future
 releases.
 @end table
 
-@item -fgnu89-inline
-@opindex fgnu89-inline
-The option @option{-fgnu89-inline} tells GCC to use the traditional
-GNU semantics for @code{inline} functions when in C99 mode.
-@xref{Inline,,An Inline Function is As Fast As a Macro}.
-Using this option is roughly equivalent to adding the
-@code{gnu_inline} function attribute to all inline functions
-(@pxref{Function Attributes}).
-
-The option @option{-fno-gnu89-inline} explicitly tells GCC to use the
-C99 semantics for @code{inline} when in C99 or gnu99 mode (i.e., it
-specifies the default behavior).
-This option is not supported in @option{-std=c90} or
-@option{-std=gnu90} mode.
-
-The preprocessor macros @code{__GNUC_GNU_INLINE__} and
-@code{__GNUC_STDC_INLINE__} may be used to check which semantics are
-in effect for @code{inline} functions.  @xref{Common Predefined
-Macros,,,cpp,The C Preprocessor}.
-
-@item -fpermitted-flt-eval-methods=@var{style}
-@opindex fpermitted-flt-eval-methods
-@opindex fpermitted-flt-eval-methods=c11
-@opindex fpermitted-flt-eval-methods=ts-18661-3
-ISO/IEC TS 18661-3 defines new permissible values for
-@code{FLT_EVAL_METHOD} that indicate that operations and constants with
-a semantic type that is an interchange or extended format should be
-evaluated to the precision and range of that type.  These new values are
-a superset of those permitted under C99/C11, which does not specify the
-meaning of other positive values of @code{FLT_EVAL_METHOD}.  As such, code
-conforming to C11 may not have been written expecting the possibility of
-the new values.
-
-@option{-fpermitted-flt-eval-methods} specifies whether the compiler
-should allow only the values of @code{FLT_EVAL_METHOD} specified in C99/C11,
-or the extended set of values specified in ISO/IEC TS 18661-3.
-
-@var{style} is either @code{c11} or @code{ts-18661-3} as appropriate.
-
-The default when in a standards compliant mode (@option{-std=c11} or similar)
-is @option{-fpermitted-flt-eval-methods=c11}.  The default when in a GNU
-dialect (@option{-std=gnu11} or similar) is
-@option{-fpermitted-flt-eval-methods=ts-18661-3}.
-
 @item -aux-info @var{filename}
 @opindex aux-info
 Output to the given filename prototyped declarations for all functions
@@ -2572,6 +2528,25 @@ built-in functions selectively when using @option{-fno-builtin} or
 #define strcpy(d, s)    __builtin_strcpy ((d), (s))
 @end smallexample
 
+@item -fcond-mismatch
+@opindex fcond-mismatch
+Allow conditional expressions with mismatched types in the second and
+third arguments.  The value of such an expression is void.  This option
+is not supported for C++.
+
+@item -ffreestanding
+@opindex ffreestanding
+@cindex hosted environment
+
+Assert that compilation targets a freestanding environment.  This
+implies @option{-fno-builtin}.  A freestanding environment
+is one in which the standard library may not exist, and program startup may
+not necessarily be at @code{main}.  The most obvious example is an OS kernel.
+This is equivalent to @option{-fno-hosted}.
+
+@xref{Standards,,Language Standards Supported by GCC}, for details of
+freestanding and hosted environments.
+
 @item -fgimple
 @opindex fgimple
 
@@ -2579,6 +2554,42 @@ Enable parsing of function definitions marked with @code{__GIMPLE}.
 This is an experimental feature that allows unit testing of GIMPLE
 passes.
 
+@item -fgnu-tm
+@opindex fgnu-tm
+When the option @option{-fgnu-tm} is specified, the compiler
+generates code for the Linux variant of Intel's current Transactional
+Memory ABI specification document (Revision 1.1, May 6 2009).  This is
+an experimental feature whose interface may change in future versions
+of GCC, as the official specification changes.  Please note that not
+all architectures are supported for this feature.
+
+For more information on GCC's support for transactional memory,
+@xref{Enabling libitm,,The GNU Transactional Memory Library,libitm,GNU
+Transactional Memory Library}.
+
+Note that the transactional memory feature is not supported with
+non-call exceptions (@option{-fnon-call-exceptions}).
+
+@item -fgnu89-inline
+@opindex fgnu89-inline
+The option @option{-fgnu89-inline} tells GCC to use the traditional
+GNU semantics for @code{inline} functions when in C99 mode.
+@xref{Inline,,An Inline Function is As Fast As a Macro}.
+Using this option is roughly equivalent to adding the
+@code{gnu_inline} function attribute to all inline functions
+(@pxref{Function Attributes}).
+
+The option @option{-fno-gnu89-inline} explicitly tells GCC to use the
+C99 semantics for @code{inline} when in C99 or gnu99 mode (i.e., it
+specifies the default behavior).
+This option is not supported in @option{-std=c90} or
+@option{-std=gnu90} mode.
+
+The preprocessor macros @code{__GNUC_GNU_INLINE__} and
+@code{__GNUC_STDC_INLINE__} may be used to check which semantics are
+in effect for @code{inline} functions.  @xref{Common Predefined
+Macros,,,cpp,The C Preprocessor}.
+
 @item -fhosted
 @opindex fhosted
 @cindex hosted environment
@@ -2589,18 +2600,32 @@ entire standard library is available, and in which @code{main} has a return
 type of @code{int}.  Examples are nearly everything except a kernel.
 This is equivalent to @option{-fno-freestanding}.
 
-@item -ffreestanding
-@opindex ffreestanding
-@cindex hosted environment
+@item -flax-vector-conversions
+@opindex flax-vector-conversions
+Allow implicit conversions between vectors with differing numbers of
+elements and/or incompatible element types.  This option should not be
+used for new code.
 
-Assert that compilation targets a freestanding environment.  This
-implies @option{-fno-builtin}.  A freestanding environment
-is one in which the standard library may not exist, and program startup may
-not necessarily be at @code{main}.  The most obvious example is an OS kernel.
-This is equivalent to @option{-fno-hosted}.
+@item -fms-extensions
+@opindex fms-extensions
+Accept some non-standard constructs used in Microsoft header files.
 
-@xref{Standards,,Language Standards Supported by GCC}, for details of
-freestanding and hosted environments.
+In C++ code, this allows member names in structures to be similar
+to previous types declarations.
+
+@smallexample
+typedef int UOW;
+struct ABC @{
+  UOW UOW;
+@};
+@end smallexample
+
+Some cases of unnamed fields in structures and unions are only
+accepted with this option.  @xref{Unnamed Fields,,Unnamed struct/union
+fields within structs/unions}, for details.
+
+Note that this option is o?f for all targets except for x86
+targets using ms-abi.
 
 @item -fopenacc
 @opindex fopenacc
@@ -2639,42 +2664,58 @@ 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 -fgnu-tm
-@opindex fgnu-tm
-When the option @option{-fgnu-tm} is specified, the compiler
-generates code for the Linux variant of Intel's current Transactional
-Memory ABI specification document (Revision 1.1, May 6 2009).  This is
-an experimental feature whose interface may change in future versions
-of GCC, as the official specification changes.  Please note that not
-all architectures are supported for this feature.
-
-For more information on GCC's support for transactional memory,
-@xref{Enabling libitm,,The GNU Transactional Memory Library,libitm,GNU
-Transactional Memory Library}.
-
-Note that the transactional memory feature is not supported with
-non-call exceptions (@option{-fnon-call-exceptions}).
-
-@item -fms-extensions
-@opindex fms-extensions
-Accept some non-standard constructs used in Microsoft header files.
-
-In C++ code, this allows member names in structures to be similar
-to previous types declarations.
+@item -foffload=disable
+@itemx -foffload=default
+@itemx -foffload=@var{target-list}
+@opindex foffload
+@cindex Offloading targets
+@cindex OpenACC offloading targets
+@cindex OpenMP offloading targets
+Specify for which OpenMP and OpenACC offload targets code should be generated.
+The default behavior, equivalent to @option{-foffload=default}, is to generate
+code for all supported offload targets.  The @option{-foffload=disable} form
+generates code only for the host fallback, while
+@option{-foffload=@var{target-list}} generates code only for the specified
+comma-separated list of offload targets. 
+
+Offload targets are specified in GCC's internal target-triplet format. You can
+run the compiler with @option{-v} to show the list of configured offload targets
+under @code{OFFLOAD_TARGET_NAMES}. 
+
+@item -foffload-options=@var{options}
+@itemx -foffload-options=@var{target-triplet-list}=@var{options}
+@opindex foffload
+@cindex Offloading options
+@cindex OpenACC offloading options
+@cindex OpenMP offloading options
+
+With @option{-foffload-options=@var{options}}, GCC passes the specified
+@var{options} to the compilers for all enabled offloading targets.  You can
+specify options that apply only to a specific target or targets by using
+the @option{-foffload-options=@var{target-list}=@var{options}} form.  The
+@var{target-list} is a comma-separated list in the same format as for the
+@option{-foffload=} option. 
+
+Typical command lines are
 
 @smallexample
-typedef int UOW;
-struct ABC @{
-  UOW UOW;
-@};
+-foffload-options=-lgfortran -foffload-options=-lm
+-foffload-options="-lgfortran -lm" -foffload-options=nvptx-none=-latomic
+-foffload-options=amdgcn-amdhsa=-march=gfx906 -foffload-options=-lm
 @end smallexample
 
-Some cases of unnamed fields in structures and unions are only
-accepted with this option.  @xref{Unnamed Fields,,Unnamed struct/union
-fields within structs/unions}, for details.
-
-Note that this option is off for all targets except for x86
-targets using ms-abi.
+@item -fpermitted-flt-eval-methods=@var{style}
+@opindex fpermitted-flt-eval-methods
+@opindex fpermitted-flt-eval-methods=c11
+@opindex fpermitted-flt-eval-methods=ts-18661-3
+ISO/IEC TS 18661-3 defines new permissible values for
+@code{FLT_EVAL_METHOD} that indicate that operations and constants with
+a semantic type that is an interchange or extended format should be
+evaluated to the precision and range of that type.  These new values are
+a superset of those permitted under C99/C11, which does not specify the
+meaning of other positive values of @code{FLT_EVAL_METHOD}.  As such, code
+conforming to C11 may not have been written expecting the possibility of
+the new values.
 
 @item -fplan9-extensions
 @opindex fplan9-extensions
@@ -2687,17 +2728,26 @@ fields declared using a typedef.  @xref{Unnamed Fields,,Unnamed
 struct/union fields within structs/unions}, for details.  This is only
 supported for C, not C++.
 
-@item -fcond-mismatch
-@opindex fcond-mismatch
-Allow conditional expressions with mismatched types in the second and
-third arguments.  The value of such an expression is void.  This option
-is not supported for C++.
+@item -fsigned-bitfields
+@itemx -funsigned-bitfields
+@itemx -fno-signed-bitfields
+@itemx -fno-unsigned-bitfields
+@opindex fsigned-bitfields
+@opindex funsigned-bitfields
+@opindex fno-signed-bitfields
+@opindex fno-unsigned-bitfields
+These options control whether a bit-field is signed or unsigned, when the
+declaration does not use either @code{signed} or @code{unsigned}.  By
+default, such a bit-field is signed, because this is consistent: the
+basic integer types such as @code{int} are signed types.
 
-@item -flax-vector-conversions
-@opindex flax-vector-conversions
-Allow implicit conversions between vectors with differing numbers of
-elements and/or incompatible element types.  This option should not be
-used for new code.
+@item -fsigned-char
+@opindex fsigned-char
+Let the type @code{char} be signed, like @code{signed char}.
+
+Note that this is equivalent to @option{-fno-unsigned-char}, which is
+the negative form of @option{-funsigned-char}.  Likewise, the option
+@option{-fno-signed-char} is equivalent to @option{-funsigned-char}.
 
 @item -funsigned-char
 @opindex funsigned-char
@@ -2718,27 +2768,6 @@ The type @code{char} is always a distinct type from each of
 @code{signed char} or @code{unsigned char}, even though its behavior
 is always just like one of those two.
 
-@item -fsigned-char
-@opindex fsigned-char
-Let the type @code{char} be signed, like @code{signed char}.
-
-Note that this is equivalent to @option{-fno-unsigned-char}, which is
-the negative form of @option{-funsigned-char}.  Likewise, the option
-@option{-fno-signed-char} is equivalent to @option{-funsigned-char}.
-
-@item -fsigned-bitfields
-@itemx -funsigned-bitfields
-@itemx -fno-signed-bitfields
-@itemx -fno-unsigned-bitfields
-@opindex fsigned-bitfields
-@opindex funsigned-bitfields
-@opindex fno-signed-bitfields
-@opindex fno-unsigned-bitfields
-These options control whether a bit-field is signed or unsigned, when the
-declaration does not use either @code{signed} or @code{unsigned}.  By
-default, such a bit-field is signed, because this is consistent: the
-basic integer types such as @code{int} are signed types.
-
 @item -fsso-struct=@var{endianness}
 @opindex fsso-struct
 Set the default scalar storage order of structures and unions to the
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 479ba6fa6ce..8bd7bdaf693 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -4531,22 +4531,28 @@ convert_CFI_desc (gfc_wrapped_block * block, gfc_symbol *sym)
       gfc_add_expr_to_block (&outer_block, incoming);
       incoming = gfc_finish_block (&outer_block);
 
-
       /* Convert the gfc descriptor back to the CFI type before going
 	 out of scope, if the CFI type was present at entry.  */
-      gfc_init_block (&outer_block);
-      gfc_init_block (&tmpblock);
-
-      tmp = gfc_build_addr_expr (ppvoid_type_node, CFI_desc_ptr);
-      outgoing = build_call_expr_loc (input_location,
-			gfor_fndecl_gfc_to_cfi, 2, tmp, gfc_desc_ptr);
-      gfc_add_expr_to_block (&tmpblock, outgoing);
+      outgoing = NULL_TREE;
+      if ((sym->attr.pointer || sym->attr.allocatable)
+	  && !sym->attr.value
+	  && sym->attr.intent != INTENT_IN)
+	{
+	  gfc_init_block (&outer_block);
+	  gfc_init_block (&tmpblock);
 
-      outgoing = build3_v (COND_EXPR, present,
-			   gfc_finish_block (&tmpblock),
-			   build_empty_stmt (input_location));
-      gfc_add_expr_to_block (&outer_block, outgoing);
-      outgoing = gfc_finish_block (&outer_block);
+	  tmp = gfc_build_addr_expr (ppvoid_type_node, CFI_desc_ptr);
+	  outgoing = build_call_expr_loc (input_location,
+					  gfor_fndecl_gfc_to_cfi, 2,
+					  tmp, gfc_desc_ptr);
+	  gfc_add_expr_to_block (&tmpblock, outgoing);
+
+	  outgoing = build3_v (COND_EXPR, present,
+			       gfc_finish_block (&tmpblock),
+			       build_empty_stmt (input_location));
+	  gfc_add_expr_to_block (&outer_block, outgoing);
+	  outgoing = gfc_finish_block (&outer_block);
+	}
 
       /* Add the lot to the procedure init and finally blocks.  */
       gfc_add_init_cleanup (block, incoming, outgoing);
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index de406ad2e8f..73ce33185f1 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -5501,13 +5501,12 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
 	attribute = 1;
     }
 
-  /* If the formal argument is assumed shape and neither a pointer nor
-     allocatable, it is unconditionally CFI_attribute_other.  */
-  if (fsym->as->type == AS_ASSUMED_SHAPE
-      && !fsym->attr.pointer && !fsym->attr.allocatable)
-   cfi_attribute = 2;
+  if (fsym->attr.pointer)
+    cfi_attribute = 0;
+  else if (fsym->attr.allocatable)
+    cfi_attribute = 1;
   else
-   cfi_attribute = attribute;
+    cfi_attribute = 2;
 
   if (e->rank != 0)
     {
@@ -5615,10 +5614,15 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
   gfc_prepend_expr_to_block (&parmse->post, tmp);
 
   /* Transfer values back to gfc descriptor.  */
-  tmp = gfc_build_addr_expr (NULL_TREE, parmse->expr);
-  tmp = build_call_expr_loc (input_location,
-			     gfor_fndecl_cfi_to_gfc, 2, gfc_desc_ptr, tmp);
-  gfc_prepend_expr_to_block (&parmse->post, tmp);
+  if (cfi_attribute != 2  /* CFI_attribute_other.  */
+      && !fsym->attr.value
+      && fsym->attr.intent != INTENT_IN)
+    {
+      tmp = gfc_build_addr_expr (NULL_TREE, parmse->expr);
+      tmp = build_call_expr_loc (input_location,
+				 gfor_fndecl_cfi_to_gfc, 2, gfc_desc_ptr, tmp);
+      gfc_prepend_expr_to_block (&parmse->post, tmp);
+    }
 
   /* Deal with an optional dummy being passed to an optional formal arg
      by finishing the pre and post blocks and making their execution
diff --git a/gcc/gcc.c b/gcc/gcc.c
index af286400a4a..203e6e5f8ed 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3977,6 +3977,68 @@ driver_wrong_lang_callback (const struct cl_decoded_option *decoded,
 static const char *spec_lang = 0;
 static int last_language_n_infiles;
 
+
+/* Check that GCC is configured to support the offload target.  */
+
+static void
+check_offload_target_name (const char *target, ptrdiff_t len)
+{
+  const char *n, *c = OFFLOAD_TARGETS;
+  char *target2 = NULL;
+  while (c)
+    {
+      n = strchr (c, ',');
+      if (n == NULL)
+	n = strchr (c, '\0');
+      if (len == n - c && strncmp (target, c, n - c) == 0)
+	break;
+      c = *n ? n + 1 : NULL;
+    }
+  if (!c)
+    {
+      if (target[len] != '\0')
+	{
+	  target2 = XNEWVEC (char, len + 1);
+	  memcpy (target2, target, len);
+	  target2[len] = '\0';
+	}
+      fatal_error (input_location,
+		 "GCC is not configured to support %qs as offload target",
+		 target2 ? target2 : target);
+      XDELETEVEC (target2);
+    }
+}
+
+/* Sanity check for -foffload-options.  */
+
+static void
+check_foffload_target_names (const char *arg)
+{
+  const char *cur, *next, *end;
+  /* If option argument starts with '-' then no target is specified and we
+     do not need to parse it.  */
+  if (arg[0] == '-')
+    return;
+  end = strchr (arg, '=');
+  if (end == NULL)
+    {
+      error ("%<=options%> missing after %<-foffload-options=target%>");
+      return;
+    }
+
+  cur = arg;
+  while (cur < end)
+    {
+      next = strchr (cur, ',');
+      if (next == NULL)
+	next = end;
+      next = (next > end) ? end : next;
+
+      check_offload_target_name (cur, next - cur);
+      cur = next + 1;
+   }
+}
+
 /* Parse -foffload option argument.  */
 
 static void
@@ -4006,33 +4068,25 @@ 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 and return, even if more targets follow. */
       if (strcmp (target, "disable") == 0)
 	{
 	  free (offload_targets);
 	  offload_targets = xstrdup ("");
-	  break;
+	  return;
 	}
 
-      /* Check that GCC is configured to support the offload target.  */
-      c = OFFLOAD_TARGETS;
-      while (c)
+      /* Reset offloading list and continue.  */
+      if (strcmp (target, "default") == 0)
 	{
-	  n = strchr (c, ',');
-	  if (n == NULL)
-	    n = strchr (c, '\0');
-
-	  if (next - cur == n - c && strncmp (target, c, n - c) == 0)
-	    break;
-
-	  c = *n ? n + 1 : NULL;
+	  free (offload_targets);
+	  offload_targets = NULL;
+	  goto next_item;
 	}
 
-      if (!c)
-	fatal_error (input_location,
-		     "GCC is not configured to support %s as offload target",
-		     target);
+      /* Check that GCC is configured to support the offload target.  */
+      check_offload_target_name (target, next - cur);
 
       if (!offload_targets)
 	{
@@ -4067,7 +4121,7 @@ handle_foffload_option (const char *arg)
 	      memcpy (offload_targets + offload_targets_len, target, next - cur + 1);
 	    }
 	}
-
+next_item:
       cur = next + 1;
       XDELETEVEC (target);
     }
@@ -4499,8 +4553,16 @@ driver_handle_option (struct gcc_options *opts,
       flag_wpa = "";
       break;
 
+    case OPT_foffload_options_:
+      check_foffload_target_names (arg);
+      break;
+
     case OPT_foffload_:
       handle_foffload_option (arg);
+      if (arg[0] == '-' || NULL != strchr (arg, '='))
+	save_switch (concat ("-foffload-options=", arg, NULL),
+		     0, NULL, validated, true);
+      do_save = false;
       break;
 
     default:
diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index 6dd55b68072..9496b3c8e0b 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -174,7 +174,8 @@ lto_write_options (void)
 	 We do not need those.  The only exception is -foffload option, if we
 	 write it in offload_lto section.  Also drop all diagnostic options.  */
       if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
-	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
+	  && (!lto_stream_offload_p
+	      || option->opt_index != OPT_foffload_options_))
 	continue;
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 1c2643984f9..aae48aff100 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -453,7 +453,7 @@ merge_and_complain (vec<cl_decoded_option> decoded_options,
 	  break;
 
 
-	case OPT_foffload_:
+	case OPT_foffload_options_:
 	  decoded_options.safe_push (*foption);
 	  break;
 
@@ -833,7 +833,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       unsigned argc;
       cl_decoded_option *option = &options[i];
 
-      if (option->opt_index != OPT_foffload_)
+      if (option->opt_index != OPT_foffload_options_)
 	continue;
 
       /* If option argument starts with '-' then no target is specified.  That
@@ -844,11 +844,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       else
 	{
 	  opts = strchr (option->arg, '=');
-	  /* If there are offload targets specified, but no actual options,
-	     there is nothing to do here.  */
-	  if (!opts)
-	    continue;
-
+	  gcc_assert (opts);
 	  cur = option->arg;
 
 	  while (cur < opts)
diff --git a/gcc/opts.c b/gcc/opts.c
index 52e9e3a9df9..3a43dd43398 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2706,7 +2706,7 @@ common_handle_option (struct gcc_options *opts,
       /* Deferred.  */
       break;
 
-    case OPT_foffload_:
+    case OPT_foffload_options_:
       /* Deferred.  */
       break;
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-28 11:28                   ` Tobias Burnus
@ 2021-06-28 15:51                     ` Tobias Burnus
  2021-06-28 22:34                       ` Sandra Loosemore
  2021-06-29 11:58                       ` Jakub Jelinek
  0 siblings, 2 replies; 19+ messages in thread
From: Tobias Burnus @ 2021-06-28 15:51 UTC (permalink / raw)
  To: Sandra Loosemore, Jakub Jelinek
  Cc: Richard Biener, gcc-patches, Thomas Schwinge

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

I managed to delete the libgomp part before posting the patch, hence,
reposted.

(The change from -foffload= to -foffload-options= ensures that also
other configured compilers such as GCN are used, an issue that Thomas
found. The original -foffload=nvptx-none=-latomic was added because as
otherwise the GCN part caused build issues for Richard.)

Thus, this patch is like v3, except for the invoke.texi fixes suggested
by Sandra (thanks!) + adding a ChangeLog
and like v4, except the lost libgomp changes has been re-added (+
ChangeLog update).

I hope it now is fine.

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-v5.diff --]
[-- Type: text/x-patch, Size: 25974 bytes --]

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

As -foffload={options,targets,targets=options} is very convoluted,
it has been split into -foffload=targets (supporting the old syntax
for backward compatibilty) and -foffload-options={options,target=options}.

Only the new syntax is documented.

Additionally, -foffload=default is supported, which can reset the
devices after -foffload=disable / -foffload=targets to the default,
if needed.

gcc/ChangeLog:

        * common.opt (-foffload=): Update description.
	(-foffload-options=): New.
        * doc/invoke.texi (C Language Options): Sort options
	alphabetical in optlist and also the description itself.
	(-foffload, -foffload-options): New.
        * gcc.c (check_offload_target_name): New, split off from
	handle_foffload_option.
        (check_foffload_target_names): New.
        (handle_foffload_option): Handle -foffload=default.
        (driver_handle_option): Update for -foffload-options.
        * lto-opts.c (lto_write_options): Use -foffload-options
	instead of -foffload.
        * lto-wrapper.c (merge_and_complain, append_offload_options):
	Likewise.
        * opts.c (common_handle_option): Likewise.

libgomp/ChangeLog:

        * testsuite/libgomp.c-c++-common/reduction-16.c: Replace
	-foffload=nvptx-none= by -foffload-options=nvptx-none= to
	avoid disabling other offload targets.
        * 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/common.opt                                     |  10 +-
 gcc/doc/invoke.texi                                | 281 ++++++++++++---------
 gcc/gcc.c                                          | 100 ++++++--
 gcc/lto-opts.c                                     |   3 +-
 gcc/lto-wrapper.c                                  |  10 +-
 gcc/opts.c                                         |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-16.c  |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-5.c   |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-6.c   |   2 +-
 libgomp/testsuite/libgomp.c/target-44.c            |   2 +-
 10 files changed, 254 insertions(+), 160 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index a1353e06bdc..a695a8c5964 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2095,9 +2095,15 @@ fnon-call-exceptions
 Common Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions.
 
+; -foffload=<targets> is documented
+; -foffload=<targets>=<options> is supported for backward compatibility
 foffload=
-Common Driver Joined MissingArgError(options or targets missing after %qs)
--foffload=<targets>=<options>	Specify offloading targets and options for them.
+Driver Joined MissingArgError(targets missing after %qs)
+-foffload=<targets>	Specify offloading targets
+
+foffload-options=
+Common Driver Joined MissingArgError(options or targets=options missing after %qs)
+-foffload=<targets>=<options>	Specify options for the offloading targets
 
 foffload-abi=
 Common Joined RejectNegative Enum(offload_abi) Var(flag_offload_abi) Init(OFFLOAD_ABI_UNSET)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index af2ce189fae..f8e41d41801 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -197,17 +197,17 @@ in the following sections.
 
 @item C Language Options
 @xref{C Dialect Options,,Options Controlling C Dialect}.
-@gccoptlist{-ansi  -std=@var{standard}  -fgnu89-inline @gol
--fpermitted-flt-eval-methods=@var{standard} @gol
--aux-info @var{filename}  -fallow-parameterless-variadic-functions @gol
--fno-asm  -fno-builtin  -fno-builtin-@var{function}  -fgimple@gol
--fhosted  -ffreestanding @gol
+@gccoptlist{-ansi  -std=@var{standard}  -aux-info @var{filename} @gol
+-fallow-parameterless-variadic-functions  -fno-asm  @gol
+-fno-builtin  -fno-builtin-@var{function}  -fcond-mismatch @gol
+-ffreestanding  -fgimple  -fgnu-tm  -fgnu89-inline  -fhosted @gol
+-flax-vector-conversions  -fms-extensions @gol
 -fopenacc  -fopenacc-dim=@var{geom} @gol
+-foffload=@var{arg} -foffload-options=@var{arg} @gol
 -fopenmp  -fopenmp-simd @gol
--fms-extensions  -fplan9-extensions  -fsso-struct=@var{endianness} @gol
--fallow-single-precision  -fcond-mismatch  -flax-vector-conversions @gol
--fsigned-bitfields  -fsigned-char @gol
--funsigned-bitfields  -funsigned-char}
+-fpermitted-flt-eval-methods=@var{standard} @gol
+-fplan9-extensions -fsigned-bitfields -funsigned-bitfields @gol
+-fsigned-char -funsigned-char -fsso-struct=@var{endianness}}
 
 @item C++ Language Options
 @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
@@ -2448,50 +2448,6 @@ and will almost certainly change in incompatible ways in future
 releases.
 @end table
 
-@item -fgnu89-inline
-@opindex fgnu89-inline
-The option @option{-fgnu89-inline} tells GCC to use the traditional
-GNU semantics for @code{inline} functions when in C99 mode.
-@xref{Inline,,An Inline Function is As Fast As a Macro}.
-Using this option is roughly equivalent to adding the
-@code{gnu_inline} function attribute to all inline functions
-(@pxref{Function Attributes}).
-
-The option @option{-fno-gnu89-inline} explicitly tells GCC to use the
-C99 semantics for @code{inline} when in C99 or gnu99 mode (i.e., it
-specifies the default behavior).
-This option is not supported in @option{-std=c90} or
-@option{-std=gnu90} mode.
-
-The preprocessor macros @code{__GNUC_GNU_INLINE__} and
-@code{__GNUC_STDC_INLINE__} may be used to check which semantics are
-in effect for @code{inline} functions.  @xref{Common Predefined
-Macros,,,cpp,The C Preprocessor}.
-
-@item -fpermitted-flt-eval-methods=@var{style}
-@opindex fpermitted-flt-eval-methods
-@opindex fpermitted-flt-eval-methods=c11
-@opindex fpermitted-flt-eval-methods=ts-18661-3
-ISO/IEC TS 18661-3 defines new permissible values for
-@code{FLT_EVAL_METHOD} that indicate that operations and constants with
-a semantic type that is an interchange or extended format should be
-evaluated to the precision and range of that type.  These new values are
-a superset of those permitted under C99/C11, which does not specify the
-meaning of other positive values of @code{FLT_EVAL_METHOD}.  As such, code
-conforming to C11 may not have been written expecting the possibility of
-the new values.
-
-@option{-fpermitted-flt-eval-methods} specifies whether the compiler
-should allow only the values of @code{FLT_EVAL_METHOD} specified in C99/C11,
-or the extended set of values specified in ISO/IEC TS 18661-3.
-
-@var{style} is either @code{c11} or @code{ts-18661-3} as appropriate.
-
-The default when in a standards compliant mode (@option{-std=c11} or similar)
-is @option{-fpermitted-flt-eval-methods=c11}.  The default when in a GNU
-dialect (@option{-std=gnu11} or similar) is
-@option{-fpermitted-flt-eval-methods=ts-18661-3}.
-
 @item -aux-info @var{filename}
 @opindex aux-info
 Output to the given filename prototyped declarations for all functions
@@ -2572,6 +2528,25 @@ built-in functions selectively when using @option{-fno-builtin} or
 #define strcpy(d, s)    __builtin_strcpy ((d), (s))
 @end smallexample
 
+@item -fcond-mismatch
+@opindex fcond-mismatch
+Allow conditional expressions with mismatched types in the second and
+third arguments.  The value of such an expression is void.  This option
+is not supported for C++.
+
+@item -ffreestanding
+@opindex ffreestanding
+@cindex hosted environment
+
+Assert that compilation targets a freestanding environment.  This
+implies @option{-fno-builtin}.  A freestanding environment
+is one in which the standard library may not exist, and program startup may
+not necessarily be at @code{main}.  The most obvious example is an OS kernel.
+This is equivalent to @option{-fno-hosted}.
+
+@xref{Standards,,Language Standards Supported by GCC}, for details of
+freestanding and hosted environments.
+
 @item -fgimple
 @opindex fgimple
 
@@ -2579,6 +2554,42 @@ Enable parsing of function definitions marked with @code{__GIMPLE}.
 This is an experimental feature that allows unit testing of GIMPLE
 passes.
 
+@item -fgnu-tm
+@opindex fgnu-tm
+When the option @option{-fgnu-tm} is specified, the compiler
+generates code for the Linux variant of Intel's current Transactional
+Memory ABI specification document (Revision 1.1, May 6 2009).  This is
+an experimental feature whose interface may change in future versions
+of GCC, as the official specification changes.  Please note that not
+all architectures are supported for this feature.
+
+For more information on GCC's support for transactional memory,
+@xref{Enabling libitm,,The GNU Transactional Memory Library,libitm,GNU
+Transactional Memory Library}.
+
+Note that the transactional memory feature is not supported with
+non-call exceptions (@option{-fnon-call-exceptions}).
+
+@item -fgnu89-inline
+@opindex fgnu89-inline
+The option @option{-fgnu89-inline} tells GCC to use the traditional
+GNU semantics for @code{inline} functions when in C99 mode.
+@xref{Inline,,An Inline Function is As Fast As a Macro}.
+Using this option is roughly equivalent to adding the
+@code{gnu_inline} function attribute to all inline functions
+(@pxref{Function Attributes}).
+
+The option @option{-fno-gnu89-inline} explicitly tells GCC to use the
+C99 semantics for @code{inline} when in C99 or gnu99 mode (i.e., it
+specifies the default behavior).
+This option is not supported in @option{-std=c90} or
+@option{-std=gnu90} mode.
+
+The preprocessor macros @code{__GNUC_GNU_INLINE__} and
+@code{__GNUC_STDC_INLINE__} may be used to check which semantics are
+in effect for @code{inline} functions.  @xref{Common Predefined
+Macros,,,cpp,The C Preprocessor}.
+
 @item -fhosted
 @opindex fhosted
 @cindex hosted environment
@@ -2589,18 +2600,32 @@ entire standard library is available, and in which @code{main} has a return
 type of @code{int}.  Examples are nearly everything except a kernel.
 This is equivalent to @option{-fno-freestanding}.
 
-@item -ffreestanding
-@opindex ffreestanding
-@cindex hosted environment
+@item -flax-vector-conversions
+@opindex flax-vector-conversions
+Allow implicit conversions between vectors with differing numbers of
+elements and/or incompatible element types.  This option should not be
+used for new code.
 
-Assert that compilation targets a freestanding environment.  This
-implies @option{-fno-builtin}.  A freestanding environment
-is one in which the standard library may not exist, and program startup may
-not necessarily be at @code{main}.  The most obvious example is an OS kernel.
-This is equivalent to @option{-fno-hosted}.
+@item -fms-extensions
+@opindex fms-extensions
+Accept some non-standard constructs used in Microsoft header files.
 
-@xref{Standards,,Language Standards Supported by GCC}, for details of
-freestanding and hosted environments.
+In C++ code, this allows member names in structures to be similar
+to previous types declarations.
+
+@smallexample
+typedef int UOW;
+struct ABC @{
+  UOW UOW;
+@};
+@end smallexample
+
+Some cases of unnamed fields in structures and unions are only
+accepted with this option.  @xref{Unnamed Fields,,Unnamed struct/union
+fields within structs/unions}, for details.
+
+Note that this option is o?f for all targets except for x86
+targets using ms-abi.
 
 @item -fopenacc
 @opindex fopenacc
@@ -2639,42 +2664,58 @@ 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 -fgnu-tm
-@opindex fgnu-tm
-When the option @option{-fgnu-tm} is specified, the compiler
-generates code for the Linux variant of Intel's current Transactional
-Memory ABI specification document (Revision 1.1, May 6 2009).  This is
-an experimental feature whose interface may change in future versions
-of GCC, as the official specification changes.  Please note that not
-all architectures are supported for this feature.
-
-For more information on GCC's support for transactional memory,
-@xref{Enabling libitm,,The GNU Transactional Memory Library,libitm,GNU
-Transactional Memory Library}.
-
-Note that the transactional memory feature is not supported with
-non-call exceptions (@option{-fnon-call-exceptions}).
-
-@item -fms-extensions
-@opindex fms-extensions
-Accept some non-standard constructs used in Microsoft header files.
-
-In C++ code, this allows member names in structures to be similar
-to previous types declarations.
+@item -foffload=disable
+@itemx -foffload=default
+@itemx -foffload=@var{target-list}
+@opindex foffload
+@cindex Offloading targets
+@cindex OpenACC offloading targets
+@cindex OpenMP offloading targets
+Specify for which OpenMP and OpenACC offload targets code should be generated.
+The default behavior, equivalent to @option{-foffload=default}, is to generate
+code for all supported offload targets.  The @option{-foffload=disable} form
+generates code only for the host fallback, while
+@option{-foffload=@var{target-list}} generates code only for the specified
+comma-separated list of offload targets. 
+
+Offload targets are specified in GCC's internal target-triplet format. You can
+run the compiler with @option{-v} to show the list of configured offload targets
+under @code{OFFLOAD_TARGET_NAMES}. 
+
+@item -foffload-options=@var{options}
+@itemx -foffload-options=@var{target-triplet-list}=@var{options}
+@opindex foffload
+@cindex Offloading options
+@cindex OpenACC offloading options
+@cindex OpenMP offloading options
+
+With @option{-foffload-options=@var{options}}, GCC passes the specified
+@var{options} to the compilers for all enabled offloading targets.  You can
+specify options that apply only to a specific target or targets by using
+the @option{-foffload-options=@var{target-list}=@var{options}} form.  The
+@var{target-list} is a comma-separated list in the same format as for the
+@option{-foffload=} option. 
+
+Typical command lines are
 
 @smallexample
-typedef int UOW;
-struct ABC @{
-  UOW UOW;
-@};
+-foffload-options=-lgfortran -foffload-options=-lm
+-foffload-options="-lgfortran -lm" -foffload-options=nvptx-none=-latomic
+-foffload-options=amdgcn-amdhsa=-march=gfx906 -foffload-options=-lm
 @end smallexample
 
-Some cases of unnamed fields in structures and unions are only
-accepted with this option.  @xref{Unnamed Fields,,Unnamed struct/union
-fields within structs/unions}, for details.
-
-Note that this option is off for all targets except for x86
-targets using ms-abi.
+@item -fpermitted-flt-eval-methods=@var{style}
+@opindex fpermitted-flt-eval-methods
+@opindex fpermitted-flt-eval-methods=c11
+@opindex fpermitted-flt-eval-methods=ts-18661-3
+ISO/IEC TS 18661-3 defines new permissible values for
+@code{FLT_EVAL_METHOD} that indicate that operations and constants with
+a semantic type that is an interchange or extended format should be
+evaluated to the precision and range of that type.  These new values are
+a superset of those permitted under C99/C11, which does not specify the
+meaning of other positive values of @code{FLT_EVAL_METHOD}.  As such, code
+conforming to C11 may not have been written expecting the possibility of
+the new values.
 
 @item -fplan9-extensions
 @opindex fplan9-extensions
@@ -2687,17 +2728,26 @@ fields declared using a typedef.  @xref{Unnamed Fields,,Unnamed
 struct/union fields within structs/unions}, for details.  This is only
 supported for C, not C++.
 
-@item -fcond-mismatch
-@opindex fcond-mismatch
-Allow conditional expressions with mismatched types in the second and
-third arguments.  The value of such an expression is void.  This option
-is not supported for C++.
+@item -fsigned-bitfields
+@itemx -funsigned-bitfields
+@itemx -fno-signed-bitfields
+@itemx -fno-unsigned-bitfields
+@opindex fsigned-bitfields
+@opindex funsigned-bitfields
+@opindex fno-signed-bitfields
+@opindex fno-unsigned-bitfields
+These options control whether a bit-field is signed or unsigned, when the
+declaration does not use either @code{signed} or @code{unsigned}.  By
+default, such a bit-field is signed, because this is consistent: the
+basic integer types such as @code{int} are signed types.
 
-@item -flax-vector-conversions
-@opindex flax-vector-conversions
-Allow implicit conversions between vectors with differing numbers of
-elements and/or incompatible element types.  This option should not be
-used for new code.
+@item -fsigned-char
+@opindex fsigned-char
+Let the type @code{char} be signed, like @code{signed char}.
+
+Note that this is equivalent to @option{-fno-unsigned-char}, which is
+the negative form of @option{-funsigned-char}.  Likewise, the option
+@option{-fno-signed-char} is equivalent to @option{-funsigned-char}.
 
 @item -funsigned-char
 @opindex funsigned-char
@@ -2718,27 +2768,6 @@ The type @code{char} is always a distinct type from each of
 @code{signed char} or @code{unsigned char}, even though its behavior
 is always just like one of those two.
 
-@item -fsigned-char
-@opindex fsigned-char
-Let the type @code{char} be signed, like @code{signed char}.
-
-Note that this is equivalent to @option{-fno-unsigned-char}, which is
-the negative form of @option{-funsigned-char}.  Likewise, the option
-@option{-fno-signed-char} is equivalent to @option{-funsigned-char}.
-
-@item -fsigned-bitfields
-@itemx -funsigned-bitfields
-@itemx -fno-signed-bitfields
-@itemx -fno-unsigned-bitfields
-@opindex fsigned-bitfields
-@opindex funsigned-bitfields
-@opindex fno-signed-bitfields
-@opindex fno-unsigned-bitfields
-These options control whether a bit-field is signed or unsigned, when the
-declaration does not use either @code{signed} or @code{unsigned}.  By
-default, such a bit-field is signed, because this is consistent: the
-basic integer types such as @code{int} are signed types.
-
 @item -fsso-struct=@var{endianness}
 @opindex fsso-struct
 Set the default scalar storage order of structures and unions to the
diff --git a/gcc/gcc.c b/gcc/gcc.c
index af286400a4a..203e6e5f8ed 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3977,6 +3977,68 @@ driver_wrong_lang_callback (const struct cl_decoded_option *decoded,
 static const char *spec_lang = 0;
 static int last_language_n_infiles;
 
+
+/* Check that GCC is configured to support the offload target.  */
+
+static void
+check_offload_target_name (const char *target, ptrdiff_t len)
+{
+  const char *n, *c = OFFLOAD_TARGETS;
+  char *target2 = NULL;
+  while (c)
+    {
+      n = strchr (c, ',');
+      if (n == NULL)
+	n = strchr (c, '\0');
+      if (len == n - c && strncmp (target, c, n - c) == 0)
+	break;
+      c = *n ? n + 1 : NULL;
+    }
+  if (!c)
+    {
+      if (target[len] != '\0')
+	{
+	  target2 = XNEWVEC (char, len + 1);
+	  memcpy (target2, target, len);
+	  target2[len] = '\0';
+	}
+      fatal_error (input_location,
+		 "GCC is not configured to support %qs as offload target",
+		 target2 ? target2 : target);
+      XDELETEVEC (target2);
+    }
+}
+
+/* Sanity check for -foffload-options.  */
+
+static void
+check_foffload_target_names (const char *arg)
+{
+  const char *cur, *next, *end;
+  /* If option argument starts with '-' then no target is specified and we
+     do not need to parse it.  */
+  if (arg[0] == '-')
+    return;
+  end = strchr (arg, '=');
+  if (end == NULL)
+    {
+      error ("%<=options%> missing after %<-foffload-options=target%>");
+      return;
+    }
+
+  cur = arg;
+  while (cur < end)
+    {
+      next = strchr (cur, ',');
+      if (next == NULL)
+	next = end;
+      next = (next > end) ? end : next;
+
+      check_offload_target_name (cur, next - cur);
+      cur = next + 1;
+   }
+}
+
 /* Parse -foffload option argument.  */
 
 static void
@@ -4006,33 +4068,25 @@ 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 and return, even if more targets follow. */
       if (strcmp (target, "disable") == 0)
 	{
 	  free (offload_targets);
 	  offload_targets = xstrdup ("");
-	  break;
+	  return;
 	}
 
-      /* Check that GCC is configured to support the offload target.  */
-      c = OFFLOAD_TARGETS;
-      while (c)
+      /* Reset offloading list and continue.  */
+      if (strcmp (target, "default") == 0)
 	{
-	  n = strchr (c, ',');
-	  if (n == NULL)
-	    n = strchr (c, '\0');
-
-	  if (next - cur == n - c && strncmp (target, c, n - c) == 0)
-	    break;
-
-	  c = *n ? n + 1 : NULL;
+	  free (offload_targets);
+	  offload_targets = NULL;
+	  goto next_item;
 	}
 
-      if (!c)
-	fatal_error (input_location,
-		     "GCC is not configured to support %s as offload target",
-		     target);
+      /* Check that GCC is configured to support the offload target.  */
+      check_offload_target_name (target, next - cur);
 
       if (!offload_targets)
 	{
@@ -4067,7 +4121,7 @@ handle_foffload_option (const char *arg)
 	      memcpy (offload_targets + offload_targets_len, target, next - cur + 1);
 	    }
 	}
-
+next_item:
       cur = next + 1;
       XDELETEVEC (target);
     }
@@ -4499,8 +4553,16 @@ driver_handle_option (struct gcc_options *opts,
       flag_wpa = "";
       break;
 
+    case OPT_foffload_options_:
+      check_foffload_target_names (arg);
+      break;
+
     case OPT_foffload_:
       handle_foffload_option (arg);
+      if (arg[0] == '-' || NULL != strchr (arg, '='))
+	save_switch (concat ("-foffload-options=", arg, NULL),
+		     0, NULL, validated, true);
+      do_save = false;
       break;
 
     default:
diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index 6dd55b68072..9496b3c8e0b 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -174,7 +174,8 @@ lto_write_options (void)
 	 We do not need those.  The only exception is -foffload option, if we
 	 write it in offload_lto section.  Also drop all diagnostic options.  */
       if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
-	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
+	  && (!lto_stream_offload_p
+	      || option->opt_index != OPT_foffload_options_))
 	continue;
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 1c2643984f9..aae48aff100 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -453,7 +453,7 @@ merge_and_complain (vec<cl_decoded_option> decoded_options,
 	  break;
 
 
-	case OPT_foffload_:
+	case OPT_foffload_options_:
 	  decoded_options.safe_push (*foption);
 	  break;
 
@@ -833,7 +833,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       unsigned argc;
       cl_decoded_option *option = &options[i];
 
-      if (option->opt_index != OPT_foffload_)
+      if (option->opt_index != OPT_foffload_options_)
 	continue;
 
       /* If option argument starts with '-' then no target is specified.  That
@@ -844,11 +844,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       else
 	{
 	  opts = strchr (option->arg, '=');
-	  /* If there are offload targets specified, but no actual options,
-	     there is nothing to do here.  */
-	  if (!opts)
-	    continue;
-
+	  gcc_assert (opts);
 	  cur = option->arg;
 
 	  while (cur < opts)
diff --git a/gcc/opts.c b/gcc/opts.c
index 52e9e3a9df9..3a43dd43398 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2706,7 +2706,7 @@ common_handle_option (struct gcc_options *opts,
       /* Deferred.  */
       break;
 
-    case OPT_foffload_:
+    case OPT_foffload_options_:
       /* Deferred.  */
       break;
 
diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
index 0eea73b144b..4bf62c3828f 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target offload_target_nvptx } } */
 
 #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..52f23e35b9a 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 /* 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..62e81506bdd 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 /* 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..a5da81d7e23 100644
--- a/libgomp/testsuite/libgomp.c/target-44.c
+++ b/libgomp/testsuite/libgomp.c/target-44.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 
 #include <stdlib.h>
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-28 15:51                     ` Tobias Burnus
@ 2021-06-28 22:34                       ` Sandra Loosemore
  2021-06-29 11:58                       ` Jakub Jelinek
  1 sibling, 0 replies; 19+ messages in thread
From: Sandra Loosemore @ 2021-06-28 22:34 UTC (permalink / raw)
  To: Tobias Burnus, Jakub Jelinek; +Cc: Richard Biener, gcc-patches, Thomas Schwinge

On 6/28/21 9:51 AM, Tobias Burnus wrote:
> I managed to delete the libgomp part before posting the patch, hence, 
> reposted.
> 
> (The change from -foffload= to -foffload-options= ensures that also 
> other configured compilers such as GCN are used, an issue that Thomas 
> found. The original -foffload=nvptx-none=-latomic was added because as 
> otherwise the GCN part caused build issues for Richard.)
> 
> Thus, this patch is like v3, except for the invoke.texi fixes suggested 
> by Sandra (thanks!) + adding a ChangeLog
> and like v4, except the lost libgomp changes has been re-added (+ 
> ChangeLog update).
> 
> I hope it now is fine.

Hmmm.

> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -197,17 +197,17 @@ in the following sections.
>  
>  @item C Language Options
>  @xref{C Dialect Options,,Options Controlling C Dialect}.
> -@gccoptlist{-ansi  -std=@var{standard}  -fgnu89-inline @gol
> --fpermitted-flt-eval-methods=@var{standard} @gol
> --aux-info @var{filename}  -fallow-parameterless-variadic-functions @gol
> --fno-asm  -fno-builtin  -fno-builtin-@var{function}  -fgimple@gol
> --fhosted  -ffreestanding @gol
> +@gccoptlist{-ansi  -std=@var{standard}  -aux-info @var{filename} @gol
> +-fallow-parameterless-variadic-functions  -fno-asm  @gol
> +-fno-builtin  -fno-builtin-@var{function}  -fcond-mismatch @gol
> +-ffreestanding  -fgimple  -fgnu-tm  -fgnu89-inline  -fhosted @gol
> +-flax-vector-conversions  -fms-extensions @gol
>  -fopenacc  -fopenacc-dim=@var{geom} @gol
> +-foffload=@var{arg} -foffload-options=@var{arg} @gol

Still need two spaces between these options on the same line inside 
@gccoptlist.

>  -fopenmp  -fopenmp-simd @gol
> --fms-extensions  -fplan9-extensions  -fsso-struct=@var{endianness} @gol
> --fallow-single-precision  -fcond-mismatch  -flax-vector-conversions @gol
> --fsigned-bitfields  -fsigned-char @gol
> --funsigned-bitfields  -funsigned-char}
> +-fpermitted-flt-eval-methods=@var{standard} @gol
> +-fplan9-extensions -fsigned-bitfields -funsigned-bitfields @gol
> +-fsigned-char -funsigned-char -fsso-struct=@var{endianness}}

And on both the last two lines here.

I didn't think it was necessary to alphabetize the actual documentation 
of the options (only the table in the option summary).  I'll have to 
assume that you didn't actually change any of the text you moved around. 
  The text for -foffload and -foffload-options looks fine now.

The documentation part of the patch is OK with the whitespace changes 
(no need to post another version for me to review that).

-Sandra

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-28 15:51                     ` Tobias Burnus
  2021-06-28 22:34                       ` Sandra Loosemore
@ 2021-06-29 11:58                       ` Jakub Jelinek
  2021-06-29 13:47                         ` Tobias Burnus
  1 sibling, 1 reply; 19+ messages in thread
From: Jakub Jelinek @ 2021-06-29 11:58 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Sandra Loosemore, Richard Biener, gcc-patches, Thomas Schwinge

On Mon, Jun 28, 2021 at 05:51:30PM +0200, Tobias Burnus wrote:
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi

I think it would be better to commit the reorderings in invoke.texi
separately from the -foffload* changes, because otherwise people will keep
wondering what actually really changed.
It can go in before or after (and please take into account Sandra's
review comments).

> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -3977,6 +3977,68 @@ driver_wrong_lang_callback (const struct cl_decoded_option *decoded,
>  static const char *spec_lang = 0;
>  static int last_language_n_infiles;
>  
> +
> +/* Check that GCC is configured to support the offload target.  */
> +
> +static void
> +check_offload_target_name (const char *target, ptrdiff_t len)
> +{
> +  const char *n, *c = OFFLOAD_TARGETS;
> +  char *target2 = NULL;
> +  while (c)
> +    {
> +      n = strchr (c, ',');
> +      if (n == NULL)
> +	n = strchr (c, '\0');
> +      if (len == n - c && strncmp (target, c, n - c) == 0)
> +	break;
> +      c = *n ? n + 1 : NULL;
> +    }
> +  if (!c)
> +    {
> +      if (target[len] != '\0')
> +	{
> +	  target2 = XNEWVEC (char, len + 1);
> +	  memcpy (target2, target, len);
> +	  target2[len] = '\0';
> +	}
> +      fatal_error (input_location,
> +		 "GCC is not configured to support %qs as offload target",
> +		 target2 ? target2 : target);

Can't this be done without target2 with
      fatal_error (input_location,
		   "GCC is not configured to support %q.*s as offload target",
		   len, target);
instead, regardless if target[len] is 0 or not?

The message should be consistent between this function and handle_foffload_option
(on the q in particular).

Also, wonder if we shouldn't print the list of configured targets in that
case, see candidates_list_and_hint functions and its callers.
And it is unclear why we use fatal_error, can't unknown offload target names
be simply ignored after emitting error?

> +      XDELETEVEC (target2);
> +    }
> +}
> +
> +/* Sanity check for -foffload-options.  */
> +
> +static void
> +check_foffload_target_names (const char *arg)
> +{
> +  const char *cur, *next, *end;
> +  /* If option argument starts with '-' then no target is specified and we
> +     do not need to parse it.  */
> +  if (arg[0] == '-')
> +    return;
> +  end = strchr (arg, '=');
> +  if (end == NULL)
> +    {
> +      error ("%<=options%> missing after %<-foffload-options=target%>");

Neither options nor target are keywords, so IMHO those shouldn't appear in between
%< and %> but after the %>, so
"%<=%>options missing after %%<-foffload-options=%>target"
?

Otherwise LGTM.

	Jakub


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-29 11:58                       ` Jakub Jelinek
@ 2021-06-29 13:47                         ` Tobias Burnus
  2021-06-29 13:51                           ` Jakub Jelinek
  2021-06-29 20:47                           ` Rainer Orth
  0 siblings, 2 replies; 19+ messages in thread
From: Tobias Burnus @ 2021-06-29 13:47 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Sandra Loosemore, Richard Biener, gcc-patches, Thomas Schwinge

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

First, the doc-sorting patch has now been applied separately as

https://gcc.gnu.org/g:d479ddc0d9854905d03a3290b203a5dcb8db07eb

On 29.06.21 13:58, Jakub Jelinek wrote:

> Also, wonder if we shouldn't print the list of configured targets in that
> case, see candidates_list_and_hint functions and its callers.
> And it is unclear why we use fatal_error, can't unknown offload target names
> be simply ignored after emitting error?

Done so – as the changes now became a bit larger, I have attached the
new version of the patch – despite the LGTM.

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-v6.diff --]
[-- Type: text/x-patch, Size: 14368 bytes --]

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

As -foffload={options,targets,targets=options} is very convoluted,
it has been split into -foffload=targets (supporting the old syntax
for backward compatibilty) and -foffload-options={options,target=options}.

Only the new syntax is documented.

Additionally, -foffload=default is supported, which can reset the
devices after -foffload=disable / -foffload=targets to the default,
if needed.

gcc/ChangeLog:

        * common.opt (-foffload=): Update description.
	(-foffload-options=): New.
        * doc/invoke.texi (C Language Options): Document
	-foffload and -foffload-options.
        * gcc.c (check_offload_target_name): New, split off from
	handle_foffload_option.
        (check_foffload_target_names): New.
        (handle_foffload_option): Handle -foffload=default.
        (driver_handle_option): Update for -foffload-options.
        * lto-opts.c (lto_write_options): Use -foffload-options
	instead of -foffload.
        * lto-wrapper.c (merge_and_complain, append_offload_options):
	Likewise.
        * opts.c (common_handle_option): Likewise.

libgomp/ChangeLog:

        * testsuite/libgomp.c-c++-common/reduction-16.c: Replace
	-foffload=nvptx-none= by -foffload-options=nvptx-none= to
	avoid disabling other offload targets.
        * 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/common.opt                                     |  10 +-
 gcc/doc/invoke.texi                                |  41 +++++++
 gcc/gcc.c                                          | 121 +++++++++++++++++----
 gcc/lto-opts.c                                     |   3 +-
 gcc/lto-wrapper.c                                  |  10 +-
 gcc/opts.c                                         |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-16.c  |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-5.c   |   2 +-
 .../testsuite/libgomp.c-c++-common/reduction-6.c   |   2 +-
 libgomp/testsuite/libgomp.c/target-44.c            |   2 +-
 10 files changed, 158 insertions(+), 37 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 1dd4456e577..eaee74c580a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2100,9 +2100,15 @@ fnon-call-exceptions
 Common Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions.
 
+; -foffload=<targets> is documented
+; -foffload=<targets>=<options> is supported for backward compatibility
 foffload=
-Common Driver Joined MissingArgError(options or targets missing after %qs)
--foffload=<targets>=<options>	Specify offloading targets and options for them.
+Driver Joined MissingArgError(targets missing after %qs)
+-foffload=<targets>	Specify offloading targets
+
+foffload-options=
+Common Driver Joined MissingArgError(options or targets=options missing after %qs)
+-foffload=<targets>=<options>	Specify options for the offloading targets
 
 foffload-abi=
 Common Joined RejectNegative Enum(offload_abi) Var(flag_offload_abi) Init(OFFLOAD_ABI_UNSET)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bf529090d92..a9fd5fdc104 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -202,6 +202,7 @@ in the following sections.
 -fno-builtin  -fno-builtin-@var{function}  -fcond-mismatch @gol
 -ffreestanding  -fgimple  -fgnu-tm  -fgnu89-inline  -fhosted @gol
 -flax-vector-conversions  -fms-extensions @gol
+-foffload=@var{arg}  -foffload-options=@var{arg} @gol
 -fopenacc  -fopenacc-dim=@var{geom} @gol
 -fopenmp  -fopenmp-simd @gol
 -fpermitted-flt-eval-methods=@var{standard} @gol
@@ -2627,6 +2628,46 @@ fields within structs/unions}, for details.
 Note that this option is off for all targets except for x86
 targets using ms-abi.
 
+@item -foffload=disable
+@itemx -foffload=default
+@itemx -foffload=@var{target-list}
+@opindex foffload
+@cindex Offloading targets
+@cindex OpenACC offloading targets
+@cindex OpenMP offloading targets
+Specify for which OpenMP and OpenACC offload targets code should be generated.
+The default behavior, equivalent to @option{-foffload=default}, is to generate
+code for all supported offload targets.  The @option{-foffload=disable} form
+generates code only for the host fallback, while
+@option{-foffload=@var{target-list}} generates code only for the specified
+comma-separated list of offload targets.
+
+Offload targets are specified in GCC's internal target-triplet format. You can
+run the compiler with @option{-v} to show the list of configured offload targets
+under @code{OFFLOAD_TARGET_NAMES}.
+
+@item -foffload-options=@var{options}
+@itemx -foffload-options=@var{target-triplet-list}=@var{options}
+@opindex foffload
+@cindex Offloading options
+@cindex OpenACC offloading options
+@cindex OpenMP offloading options
+
+With @option{-foffload-options=@var{options}}, GCC passes the specified
+@var{options} to the compilers for all enabled offloading targets.  You can
+specify options that apply only to a specific target or targets by using
+the @option{-foffload-options=@var{target-list}=@var{options}} form.  The
+@var{target-list} is a comma-separated list in the same format as for the
+@option{-foffload=} option.
+
+Typical command lines are
+
+@smallexample
+-foffload-options=-lgfortran -foffload-options=-lm
+-foffload-options="-lgfortran -lm" -foffload-options=nvptx-none=-latomic
+-foffload-options=amdgcn-amdhsa=-march=gfx906 -foffload-options=-lm
+@end smallexample
+
 @item -fopenacc
 @opindex fopenacc
 @cindex OpenACC accelerator programming
diff --git a/gcc/gcc.c b/gcc/gcc.c
index af286400a4a..df9b2192873 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3977,6 +3977,84 @@ driver_wrong_lang_callback (const struct cl_decoded_option *decoded,
 static const char *spec_lang = 0;
 static int last_language_n_infiles;
 
+
+/* Check that GCC is configured to support the offload target.  */
+
+static bool
+check_offload_target_name (const char *target, ptrdiff_t len)
+{
+  const char *n, *c = OFFLOAD_TARGETS;
+  while (c)
+    {
+      n = strchr (c, ',');
+      if (n == NULL)
+	n = strchr (c, '\0');
+      if (len == n - c && strncmp (target, c, n - c) == 0)
+	break;
+      c = *n ? n + 1 : NULL;
+    }
+  if (!c)
+    {
+      char *s;
+      auto_vec<const char*> candidates;
+      char *cand = (char *) alloca (strlen (OFFLOAD_TARGETS) + 1);
+      c = OFFLOAD_TARGETS;
+      while (c)
+	{
+	  n = strchr (c, ',');
+	  if (n == NULL)
+	    n = strchr (c, '\0');
+	  strncpy (cand, c, n - c);
+	  cand[n - c] = '\0';
+	  candidates.safe_push (cand);
+	  c = *n ? n + 1 : NULL;
+	}
+      error ("GCC is not configured to support %q.*s as offload target",
+	     len, target);
+      const char *hint = candidates_list_and_hint (target, s, candidates);
+      if (hint)
+	inform (UNKNOWN_LOCATION,
+		"valid offload targets are: %s; did you mean %qs?", s, hint);
+      else
+	inform (UNKNOWN_LOCATION, "valid offload targets are: %s", s);
+      XDELETEVEC (s);
+      return false;
+    }
+  return true;
+}
+
+/* Sanity check for -foffload-options.  */
+
+static void
+check_foffload_target_names (const char *arg)
+{
+  const char *cur, *next, *end;
+  /* If option argument starts with '-' then no target is specified and we
+     do not need to parse it.  */
+  if (arg[0] == '-')
+    return;
+  end = strchr (arg, '=');
+  if (end == NULL)
+    {
+      error ("%<=%>options missing after %<-foffload-options=%>target");
+      return;
+    }
+
+  cur = arg;
+  while (cur < end)
+    {
+      next = strchr (cur, ',');
+      if (next == NULL)
+	next = end;
+      next = (next > end) ? end : next;
+
+      /* Retain non-supported targets after printing an error as those will not
+	 be processed; each enabled target only processes its triplet.  */
+      check_offload_target_name (cur, next - cur);
+      cur = next + 1;
+   }
+}
+
 /* Parse -foffload option argument.  */
 
 static void
@@ -4006,34 +4084,25 @@ 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 (strcmp (target, "disable") == 0)
+      /* Reset offloading list and continue.  */
+      if (strcmp (target, "default") == 0)
 	{
 	  free (offload_targets);
-	  offload_targets = xstrdup ("");
-	  break;
+	  offload_targets = NULL;
+	  goto next_item;
 	}
 
-      /* Check that GCC is configured to support the offload target.  */
-      c = OFFLOAD_TARGETS;
-      while (c)
+      /* If 'disable' is passed to the option, clean the list of
+	 offload targets and return, even if more targets follow.
+	 Likewise if GCC is not configured to support that offload target.  */
+      if (strcmp (target, "disable") == 0
+	  || !check_offload_target_name (target, next - cur))
 	{
-	  n = strchr (c, ',');
-	  if (n == NULL)
-	    n = strchr (c, '\0');
-
-	  if (next - cur == n - c && strncmp (target, c, n - c) == 0)
-	    break;
-
-	  c = *n ? n + 1 : NULL;
+	  free (offload_targets);
+	  offload_targets = xstrdup ("");
+	  return;
 	}
 
-      if (!c)
-	fatal_error (input_location,
-		     "GCC is not configured to support %s as offload target",
-		     target);
-
       if (!offload_targets)
 	{
 	  offload_targets = target;
@@ -4067,7 +4136,7 @@ handle_foffload_option (const char *arg)
 	      memcpy (offload_targets + offload_targets_len, target, next - cur + 1);
 	    }
 	}
-
+next_item:
       cur = next + 1;
       XDELETEVEC (target);
     }
@@ -4499,8 +4568,16 @@ driver_handle_option (struct gcc_options *opts,
       flag_wpa = "";
       break;
 
+    case OPT_foffload_options_:
+      check_foffload_target_names (arg);
+      break;
+
     case OPT_foffload_:
       handle_foffload_option (arg);
+      if (arg[0] == '-' || NULL != strchr (arg, '='))
+	save_switch (concat ("-foffload-options=", arg, NULL),
+		     0, NULL, validated, true);
+      do_save = false;
       break;
 
     default:
diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index 6dd55b68072..9496b3c8e0b 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -174,7 +174,8 @@ lto_write_options (void)
 	 We do not need those.  The only exception is -foffload option, if we
 	 write it in offload_lto section.  Also drop all diagnostic options.  */
       if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
-	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
+	  && (!lto_stream_offload_p
+	      || option->opt_index != OPT_foffload_options_))
 	continue;
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 1c2643984f9..aae48aff100 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -453,7 +453,7 @@ merge_and_complain (vec<cl_decoded_option> decoded_options,
 	  break;
 
 
-	case OPT_foffload_:
+	case OPT_foffload_options_:
 	  decoded_options.safe_push (*foption);
 	  break;
 
@@ -833,7 +833,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       unsigned argc;
       cl_decoded_option *option = &options[i];
 
-      if (option->opt_index != OPT_foffload_)
+      if (option->opt_index != OPT_foffload_options_)
 	continue;
 
       /* If option argument starts with '-' then no target is specified.  That
@@ -844,11 +844,7 @@ append_offload_options (obstack *argv_obstack, const char *target,
       else
 	{
 	  opts = strchr (option->arg, '=');
-	  /* If there are offload targets specified, but no actual options,
-	     there is nothing to do here.  */
-	  if (!opts)
-	    continue;
-
+	  gcc_assert (opts);
 	  cur = option->arg;
 
 	  while (cur < opts)
diff --git a/gcc/opts.c b/gcc/opts.c
index 66262c5b707..f159bb35130 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2724,7 +2724,7 @@ common_handle_option (struct gcc_options *opts,
       /* Deferred.  */
       break;
 
-    case OPT_foffload_:
+    case OPT_foffload_options_:
       /* Deferred.  */
       break;
 
diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
index 0eea73b144b..4bf62c3828f 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target offload_target_nvptx } } */
 
 #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..52f23e35b9a 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-5.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 /* 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..62e81506bdd 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-6.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 /* 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..a5da81d7e23 100644
--- a/libgomp/testsuite/libgomp.c/target-44.c
+++ b/libgomp/testsuite/libgomp.c/target-44.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
+/* { dg-additional-options "-foffload-options=nvptx-none=-latomic" { target { offload_target_nvptx } } } */
 
 #include <stdlib.h>
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-29 13:47                         ` Tobias Burnus
@ 2021-06-29 13:51                           ` Jakub Jelinek
  2021-06-29 20:47                           ` Rainer Orth
  1 sibling, 0 replies; 19+ messages in thread
From: Jakub Jelinek @ 2021-06-29 13:51 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: gcc-patches, Sandra Loosemore, Richard Biener, Thomas Schwinge

On Tue, Jun 29, 2021 at 03:47:03PM +0200, Tobias Burnus wrote:
> gcc/ChangeLog:
> 
>         * common.opt (-foffload=): Update description.
> 	(-foffload-options=): New.
>         * doc/invoke.texi (C Language Options): Document
> 	-foffload and -foffload-options.
>         * gcc.c (check_offload_target_name): New, split off from
> 	handle_foffload_option.
>         (check_foffload_target_names): New.
>         (handle_foffload_option): Handle -foffload=default.
>         (driver_handle_option): Update for -foffload-options.
>         * lto-opts.c (lto_write_options): Use -foffload-options
> 	instead of -foffload.
>         * lto-wrapper.c (merge_and_complain, append_offload_options):
> 	Likewise.
>         * opts.c (common_handle_option): Likewise.
> 
> libgomp/ChangeLog:
> 
>         * testsuite/libgomp.c-c++-common/reduction-16.c: Replace
> 	-foffload=nvptx-none= by -foffload-options=nvptx-none= to
> 	avoid disabling other offload targets.
>         * 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.

Ok, thanks.

	Jakub


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-29 13:47                         ` Tobias Burnus
  2021-06-29 13:51                           ` Jakub Jelinek
@ 2021-06-29 20:47                           ` Rainer Orth
  2021-06-29 21:02                             ` Christophe Lyon
  2021-06-30 20:12                             ` Rainer Orth
  1 sibling, 2 replies; 19+ messages in thread
From: Rainer Orth @ 2021-06-29 20:47 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Jakub Jelinek, gcc-patches, Sandra Loosemore, Richard Biener,
	Thomas Schwinge

Hi Tobias,

> On 29.06.21 13:58, Jakub Jelinek wrote:
>
>> Also, wonder if we shouldn't print the list of configured targets in that
>> case, see candidates_list_and_hint functions and its callers.
>> And it is unclear why we use fatal_error, can't unknown offload target names
>> be simply ignored after emitting error?
>
> Done so – as the changes now became a bit larger, I have attached the
> new version of the patch – despite the LGTM.

this patch broke Solaris bootstrap (both 32 and 64-bit sparc and x86):

/vol/gcc/src/hg/master/local/gcc/gcc.c: In function 'bool check_offload_target_name(const char*, ptrdiff_t)':
/vol/gcc/src/hg/master/local/gcc/gcc.c:4010:23: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 4010 |           cand[n - c] = '\0';
      |           ~~~~~~~~~~~~^~~~~~
In file included from /vol/gcc/src/hg/master/local/gcc/system.h:706,
                 from /vol/gcc/src/hg/master/local/gcc/gcc.c:31:
/vol/gcc/src/hg/master/local/gcc/../include/libiberty.h:733:36: note: at offset 1 into destination object of size 1 allocated by '__builtin_alloca'
  733 | # define alloca(x) __builtin_alloca(x)
      |                    ~~~~~~~~~~~~~~~~^~~
/vol/gcc/src/hg/master/local/gcc/gcc.c:4000:29: note: in expansion of macro 'alloca'
 4000 |       char *cand = (char *) alloca (strlen (OFFLOAD_TARGETS) + 1);
      |                             ^~~~~~

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-29 20:47                           ` Rainer Orth
@ 2021-06-29 21:02                             ` Christophe Lyon
  2021-06-30 20:12                             ` Rainer Orth
  1 sibling, 0 replies; 19+ messages in thread
From: Christophe Lyon @ 2021-06-29 21:02 UTC (permalink / raw)
  To: Rainer Orth
  Cc: Tobias Burnus, Jakub Jelinek, Richard Biener, Sandra Loosemore,
	gcc-patches, Thomas Schwinge

On Tue, Jun 29, 2021 at 10:48 PM Rainer Orth <ro@cebitec.uni-bielefeld.de>
wrote:

> Hi Tobias,
>
> > On 29.06.21 13:58, Jakub Jelinek wrote:
> >
> >> Also, wonder if we shouldn't print the list of configured targets in
> that
> >> case, see candidates_list_and_hint functions and its callers.
> >> And it is unclear why we use fatal_error, can't unknown offload target
> names
> >> be simply ignored after emitting error?
> >
> > Done so – as the changes now became a bit larger, I have attached the
> > new version of the patch – despite the LGTM.
>
> this patch broke Solaris bootstrap (both 32 and 64-bit sparc and x86):
>
> /vol/gcc/src/hg/master/local/gcc/gcc.c: In function 'bool
> check_offload_target_name(const char*, ptrdiff_t)':
> /vol/gcc/src/hg/master/local/gcc/gcc.c:4010:23: error: writing 1 byte into
> a region of size 0 [-Werror=stringop-overflow=]
>  4010 |           cand[n - c] = '\0';
>       |           ~~~~~~~~~~~~^~~~~~
> In file included from /vol/gcc/src/hg/master/local/gcc/system.h:706,
>                  from /vol/gcc/src/hg/master/local/gcc/gcc.c:31:
> /vol/gcc/src/hg/master/local/gcc/../include/libiberty.h:733:36: note: at
> offset 1 into destination object of size 1 allocated by '__builtin_alloca'
>   733 | # define alloca(x) __builtin_alloca(x)
>       |                    ~~~~~~~~~~~~~~~~^~~
> /vol/gcc/src/hg/master/local/gcc/gcc.c:4000:29: note: in expansion of
> macro 'alloca'
>  4000 |       char *cand = (char *) alloca (strlen (OFFLOAD_TARGETS) + 1);
>       |                             ^~~~~~
>
>         Rainer
>
>
Also seeing:
FAIL:  compiler driver --help=common option(s): "^ +-.*[^:.]$" absent from
output: "  -foffload=<targets>=<options> Specify options for the offloading
targets"

looks related to this patch.

Thanks,

Christophe

-- 
>
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch] Add 'default' to -foffload=; document that flag [PR67300]
  2021-06-29 20:47                           ` Rainer Orth
  2021-06-29 21:02                             ` Christophe Lyon
@ 2021-06-30 20:12                             ` Rainer Orth
  1 sibling, 0 replies; 19+ messages in thread
From: Rainer Orth @ 2021-06-30 20:12 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Jakub Jelinek, Richard Biener, Sandra Loosemore, gcc-patches,
	Thomas Schwinge

Hi Tobias,

> this patch broke Solaris bootstrap (both 32 and 64-bit sparc and x86):
>
> /vol/gcc/src/hg/master/local/gcc/gcc.c: In function 'bool
> check_offload_target_name(const char*, ptrdiff_t)':
> /vol/gcc/src/hg/master/local/gcc/gcc.c:4010:23: error: writing 1 byte into
> a region of size 0 [-Werror=stringop-overflow=]
>  4010 |           cand[n - c] = '\0';
>       |           ~~~~~~~~~~~~^~~~~~
> In file included from /vol/gcc/src/hg/master/local/gcc/system.h:706,
>                  from /vol/gcc/src/hg/master/local/gcc/gcc.c:31:
> /vol/gcc/src/hg/master/local/gcc/../include/libiberty.h:733:36: note: at offset 1 into destination object of size 1 allocated by '__builtin_alloca'
>   733 | # define alloca(x) __builtin_alloca(x)
>       |                    ~~~~~~~~~~~~~~~~^~~
> /vol/gcc/src/hg/master/local/gcc/gcc.c:4000:29: note: in expansion of macro
> 'alloca'
>  4000 |       char *cand = (char *) alloca (strlen (OFFLOAD_TARGETS) + 1);
>       |                             ^~~~~~

as of your next patch

commit a3ce7d75dd9c0308b8565669f31127436cb2ba9f
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Wed Jun 30 13:17:54 2021 +0200

    gcc.c's check_offload_target_name: Fixes to inform hints

Solaris bootstrap has been restored.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-06-30 20:12 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 12:08 [Patch] Add 'default' to -foffload=; document that flag [PR67300] Tobias Burnus
2021-06-17 12:27 ` Jakub Jelinek
2021-06-17 16:03   ` Tobias Burnus
2021-06-17 17:41     ` Sandra Loosemore
2021-06-17 17:50       ` Jakub Jelinek
2021-06-17 19:28         ` Tobias Burnus
2021-06-17 19:40           ` Jakub Jelinek
2021-06-17 21:57             ` Sandra Loosemore
2021-06-17 23:05               ` Tobias Burnus
2021-06-18 22:47                 ` Sandra Loosemore
2021-06-28 11:28                   ` Tobias Burnus
2021-06-28 15:51                     ` Tobias Burnus
2021-06-28 22:34                       ` Sandra Loosemore
2021-06-29 11:58                       ` Jakub Jelinek
2021-06-29 13:47                         ` Tobias Burnus
2021-06-29 13:51                           ` Jakub Jelinek
2021-06-29 20:47                           ` Rainer Orth
2021-06-29 21:02                             ` Christophe Lyon
2021-06-30 20:12                             ` Rainer Orth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).