diff mbox

[6/n] OpenMP 4.0 offloading infrastructure: option handling

Message ID 87fv25xjix.fsf@kepler.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge Sept. 23, 2015, 3:06 p.m. UTC
Hi!

I clarified the -foffload usage:
<https://gcc.gnu.org/wiki/Offloading?action=diff&rev1=38&rev2=39>.

On Wed, 23 Sep 2015 00:23:50 +0200, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 09/22/2015 02:02 PM, Thomas Schwinge wrote:
> >
> >      	gcc/
> >      	* gcc.c (handle_foffload_option): Don't lose the trailing NUL
> >      	character when appending to offload_targets.
> >
> >      	gcc/
> >      	* configure.ac (offload_targets, OFFLOAD_TARGETS): Separate
> >      	offload targets by commas, not colons.
> >      	* config.in: Regenerate.
> >      	* configure: Likewise.
> >      	* gcc.c (driver::maybe_putenv_COLLECT_LTO_WRAPPER): Due to that,
> >      	instead of setting up the default offload targets here...
> >      	(process_command): ..., do it here.
> >      	libgomp/
> >      	* plugin/configfrag.ac (OFFLOAD_TARGETS): Clarify that offload
> >      	targets are separated by commas.
> >      	* config.h.in: Regenerate.
> 
> Looks ok to me

Thanks for the prompt review!

> except this double ChangeLog seems messed up.

Hmm, I thought that was the standard way to format ChangeLogs for
several/independent changes?  Anyway, to avoid that, I've split the patch
into two separate commits; r228053 and r228054:

commit daa8f58fd840e8d35f362306fb54e1963f4cbd0f
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Sep 23 14:52:50 2015 +0000

    Fix --enable-offload-targets/-foffload handling, pt. 1
    
    	gcc/
    	* configure.ac (offload_targets, OFFLOAD_TARGETS): Separate
    	offload targets by commas, not colons.
    	* config.in: Regenerate.
    	* configure: Likewise.
    	* gcc.c (driver::maybe_putenv_COLLECT_LTO_WRAPPER): Due to that,
    	instead of setting up the default offload targets here...
    	(process_command): ..., do it here.
    	libgomp/
    	* plugin/configfrag.ac (OFFLOAD_TARGETS): Clarify that offload
    	targets are separated by commas.
    	* config.h.in: Regenerate.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@228053 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog                |   14 ++++++++++++++
 gcc/config.in                |    2 +-
 gcc/configure                |    2 +-
 gcc/configure.ac             |    4 ++--
 gcc/gcc.c                    |   23 +++++++++++++----------
 gcc/lto-wrapper.c            |    4 ++++
 libgomp/config.h.in          |    2 +-
 libgomp/plugin/configfrag.ac |    2 +-
 8 files changed, 37 insertions(+), 16 deletions(-)



Grüße,
 Thomas
diff mbox

Patch

diff --git gcc/ChangeLog gcc/ChangeLog
index 0e9b728..df71558 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,4 +1,18 @@ 
 2015-09-23  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* configure.ac (offload_targets, OFFLOAD_TARGETS): Separate
+	offload targets by commas, not colons.
+	* config.in: Regenerate.
+	* configure: Likewise.
+	* gcc.c (driver::maybe_putenv_COLLECT_LTO_WRAPPER): Due to that,
+	instead of setting up the default offload targets here...
+	(process_command): ..., do it here.
+	libgomp/
+	* plugin/configfrag.ac (OFFLOAD_TARGETS): Clarify that offload
+	targets are separated by commas.
+	* config.h.in: Regenerate.
+
+2015-09-23  Thomas Schwinge  <thomas@codesourcery.com>
 	    Nathan Sidwell  <nathan@codesourcery.com>
 
 	* omp-low.h (omp_reduction_init_op): Declare.
diff --git gcc/config.in gcc/config.in
index 431d262..c5c1be4 100644
--- gcc/config.in
+++ gcc/config.in
@@ -1913,7 +1913,7 @@ 
 #endif
 
 
-/* Define to hold the list of target names suitable for offloading. */
+/* Define to offload targets, separated by commas. */
 #ifndef USED_FOR_TARGET
 #undef OFFLOAD_TARGETS
 #endif
diff --git gcc/configure gcc/configure
index 6fb11a7..7493c80 100755
--- gcc/configure
+++ gcc/configure
@@ -7696,7 +7696,7 @@  for tgt in `echo $enable_offload_targets | sed 's/,/ /g'`; do
   if test x"$offload_targets" = x; then
     offload_targets=$tgt
   else
-    offload_targets="$offload_targets:$tgt"
+    offload_targets="$offload_targets,$tgt"
   fi
 done
 
diff --git gcc/configure.ac gcc/configure.ac
index a6e078a..9d1f6f1 100644
--- gcc/configure.ac
+++ gcc/configure.ac
@@ -941,11 +941,11 @@  for tgt in `echo $enable_offload_targets | sed 's/,/ /g'`; do
   if test x"$offload_targets" = x; then
     offload_targets=$tgt
   else
-    offload_targets="$offload_targets:$tgt"
+    offload_targets="$offload_targets,$tgt"
   fi
 done
 AC_DEFINE_UNQUOTED(OFFLOAD_TARGETS, "$offload_targets",
-  [Define to hold the list of target names suitable for offloading.])
+  [Define to offload targets, separated by commas.])
 if test x"$offload_targets" != x; then
   AC_DEFINE(ENABLE_OFFLOADING, 1,
     [Define this to enable support for offloading.])
diff --git gcc/gcc.c gcc/gcc.c
index 757bfc9..78b68e2 100644
--- gcc/gcc.c
+++ gcc/gcc.c
@@ -284,7 +284,8 @@  static const char *const spec_version = DEFAULT_TARGET_VERSION;
 static const char *spec_machine = DEFAULT_TARGET_MACHINE;
 static const char *spec_host_machine = DEFAULT_REAL_TARGET_MACHINE;
 
-/* List of offload targets.  */
+/* List of offload targets.  Separated by colon.  Empty string for
+   -foffload=disable.  */
 
 static char *offload_targets = NULL;
 
@@ -4376,6 +4377,13 @@  process_command (unsigned int decoded_options_count,
 			   CL_DRIVER, &handlers, global_dc);
     }
 
+#ifdef ENABLE_OFFLOADING
+  /* If the user didn't specify any, default to all configured offload
+     targets.  */
+  if (offload_targets == NULL)
+    handle_foffload_option (OFFLOAD_TARGETS);
+#endif
+
   if (output_file
       && strcmp (output_file, "-") != 0
       && strcmp (output_file, HOST_BIT_BUCKET) != 0)
@@ -7572,22 +7580,17 @@  driver::maybe_putenv_COLLECT_LTO_WRAPPER () const
 void
 driver::maybe_putenv_OFFLOAD_TARGETS () const
 {
-  const char *targets = offload_targets;
-
-  /* If no targets specified by -foffload, use all available targets.  */
-  if (!targets)
-    targets = OFFLOAD_TARGETS;
-
-  if (strlen (targets) > 0)
+  if (offload_targets && offload_targets[0] != '\0')
     {
       obstack_grow (&collect_obstack, "OFFLOAD_TARGET_NAMES=",
 		    sizeof ("OFFLOAD_TARGET_NAMES=") - 1);
-      obstack_grow (&collect_obstack, targets,
-		    strlen (targets) + 1);
+      obstack_grow (&collect_obstack, offload_targets,
+		    strlen (offload_targets) + 1);
       xputenv (XOBFINISH (&collect_obstack, char *));
     }
 
   free (offload_targets);
+  offload_targets = NULL;
 }
 
 /* Reject switches that no pass was interested in.  */
diff --git gcc/lto-wrapper.c gcc/lto-wrapper.c
index 150d368..e13a82a 100644
--- gcc/lto-wrapper.c
+++ gcc/lto-wrapper.c
@@ -594,6 +594,8 @@  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;
 
@@ -606,10 +608,12 @@  append_offload_options (obstack *argv_obstack, const char *target,
 		next = opts;
 	      next = (next > opts) ? opts : next;
 
+	      /* Are we looking for this offload target?  */
 	      if (strlen (target) == (size_t) (next - cur)
 		  && strncmp (target, cur, next - cur) == 0)
 		break;
 
+	      /* Skip the comma or equal sign.  */
 	      cur = next + 1;
 	    }
 
diff --git libgomp/config.h.in libgomp/config.h.in
index 8533f03..2e4c698 100644
--- libgomp/config.h.in
+++ libgomp/config.h.in
@@ -95,7 +95,7 @@ 
    */
 #undef LT_OBJDIR
 
-/* Define to hold the list of target names suitable for offloading. */
+/* Define to offload targets, separated by commas. */
 #undef OFFLOAD_TARGETS
 
 /* Name of package */
diff --git libgomp/plugin/configfrag.ac libgomp/plugin/configfrag.ac
index 8c2a420..ad70dd1 100644
--- libgomp/plugin/configfrag.ac
+++ libgomp/plugin/configfrag.ac
@@ -141,7 +141,7 @@  if test x"$enable_offload_targets" != x; then
   done
 fi
 AC_DEFINE_UNQUOTED(OFFLOAD_TARGETS, "$offload_targets",
-  [Define to hold the list of target names suitable for offloading.])
+  [Define to offload targets, separated by commas.])
 AM_CONDITIONAL([PLUGIN_NVPTX], [test $PLUGIN_NVPTX = 1])
 AC_DEFINE_UNQUOTED([PLUGIN_NVPTX], [$PLUGIN_NVPTX],
   [Define to 1 if the NVIDIA plugin is built, 0 if not.])

commit 3bf38a0b1ae8b2e8c382fa126c15ea815ac29b06
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Sep 23 14:52:57 2015 +0000

    Fix --enable-offload-targets/-foffload handling, pt. 2
    
    	gcc/
    	* gcc.c (handle_foffload_option): Don't lose the trailing NUL
    	character when appending to offload_targets.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@228054 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog |    3 +++
 gcc/gcc.c     |    7 +++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index df71558..ca89477 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,5 +1,8 @@ 
 2015-09-23  Thomas Schwinge  <thomas@codesourcery.com>
 
+	* gcc.c (handle_foffload_option): Don't lose the trailing NUL
+	character when appending to offload_targets.
+
 	* configure.ac (offload_targets, OFFLOAD_TARGETS): Separate
 	offload targets by commas, not colons.
 	* config.in: Regenerate.
diff --git gcc/gcc.c gcc/gcc.c
index 78b68e2..ef132d6 100644
--- gcc/gcc.c
+++ gcc/gcc.c
@@ -3657,10 +3657,9 @@  handle_foffload_option (const char *arg)
 	      size_t offload_targets_len = strlen (offload_targets);
 	      offload_targets
 		= XRESIZEVEC (char, offload_targets,
-			      offload_targets_len + next - cur + 2);
-	      if (offload_targets_len)
-		offload_targets[offload_targets_len++] = ':';
-	      memcpy (offload_targets + offload_targets_len, target, next - cur);
+			      offload_targets_len + 1 + next - cur + 1);
+	      offload_targets[offload_targets_len++] = ':';
+	      memcpy (offload_targets + offload_targets_len, target, next - cur + 1);
 	    }
 	}