Hi!

On Mon, 21 Sep 2015 19:41:59 +0300, Ilya Verbin <iver...@gmail.com> wrote:
> 2015-09-21 18:15 GMT+03:00 Thomas Schwinge <tho...@codesourcery.com>:
> > (<https://gcc.gnu.org/PR67300>, "--foffload* undocumented", has recently
> > been filed.)
> >
> > (In the following, "intelmic" is short for
> > "x86_64-intelmicemul-linux-gnu", and "nvptx" is short for "nvptx-none".)
> >
> > What is the syntax to use for building both intelmic and nvptx offloading
> > code?  I understand we allow for separate -foffload=intelmic
> > -foffload=nvptx options.  Do we also intend to allow
> > -foffload=intelmic,nvptx or -foffload=intelmic:nvptx?
> >
> > And then, we allow for specifying offloading compiler options with
> > -foffload=intelmic=[...] and -foffload=nvptx=[...]; do we also intend to
> > allow -foffload=intelmic,nvptx=[...] (do the options apply to nvptx only,
> > or to both intelmic and nvptx?), and/or
> > -foffload=intelmic=[...],nvptx=[...], and/or
> > -foffload=intelmic:nvptx=[...] (which "looks a bit like" the options
> > ought to apply to nvptx only -- or to both intelmic and nvptx?), and/or
> > -foffload=intelmic=[...]:nvptx=[...]?
> 
> The plan was:
> 
> 1. -foffload=intelmic,nvptx=[...]  <- apply options to both intelmic,nvptx.
>    Just like -foffload=[...] applies to both targets (if configured so).
> 2. -foffload=intelmic=[...],nvptx=[...]  <- is not allowed.
> 3. To apply different options to different targets, one should pass:
>    -foffload=intelmic=[...] -foffload=nvptx=[...].
> 
> >       3612        /* Check that GCC is configured to support the offload 
> > target.  */
> >       3613        c = OFFLOAD_TARGETS;
> >       3614        while (c)
> >       3615          {
> >       3616            n = strchr (c, ',');
> >       3617            if (n == NULL)
> >       3618              n = strchr (c, '\0');
> >       3619
> >       3620            if (next - cur == n - c && strncmp (target, c, n - c) 
> > == 0)
> >       3621              break;
> >       3622
> >       3623            c = *n ? n + 1 : NULL;
> >       3624          }
> >       3625
> >       3626        if (!c)
> >       3627          fatal_error (input_location,
> >       3628                       "GCC is not configured to support %s as 
> > offload target",
> >       3629                       target);
> >
> > So, this code will not do the right thing when configured with
> > --enable-offload-targets=intelmic,nvptx (thus,
> > OFFLOAD_TARGETS=intelmic:nvptx): using -foffload=nvptx will then result
> > in "xgcc: fatal error: GCC is not configured to support nvptx as offload
> > target".
> >
> > If I'm understanding the following code correctly, this supports the idea
> > that the intention has been for -foffload=[targets]=[options] to separate
> > the targets by commas, and separate the options by spaces -- is that
> > correct?
> 
> Yes, targets are separated by commas, options are the whole string after the
> equal sign, spaces inside are allowed.

OK.  I'm currently testing the following patch to fix this as well as a
memory corruption issue (lost NUL terminator); OK to commit once testing
finishes?

commit c72c9dc406927a20d04a29e234ed9628cef32b11
Author: Thomas Schwinge <tho...@codesourcery.com>
Date:   Mon Sep 21 17:37:04 2015 +0200

    Fix --enable-offload-targets/-foffload handling
    
        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.
---
 gcc/config.in                |    2 +-
 gcc/configure                |    2 +-
 gcc/configure.ac             |    4 ++--
 gcc/gcc.c                    |   30 ++++++++++++++++--------------
 gcc/lto-wrapper.c            |    4 ++++
 libgomp/config.h.in          |    2 +-
 libgomp/plugin/configfrag.ac |    2 +-
 7 files changed, 26 insertions(+), 20 deletions(-)

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..ef132d6 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;
 
@@ -3656,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);
            }
        }
 
@@ -4376,6 +4376,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 +7579,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.])


Grüße,
 Thomas

Attachment: signature.asc
Description: PGP signature

Reply via email to