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

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

[Ilya Verbin]

Hello,

This is the last common infrastructure patch in the series.
(Next patches will contain tests for libgomp testsuite and MIC specific things)

It introduces 2 new options:
1. -foffload=<targets>=<options>
   By default, GCC will build offload images for all offload targets specified
in configure, with non-target-specific options passed to host compiler.
This option is used to control offload targets and options for them.

It can be used in a few ways:
* -foffload=disable
  Tells GCC to disable offload support.
  OpenMP target regions will be run in host fallback mode.
* -foffload=<targets>
  Tells GCC to build offload images for <targets>.
  They will be built with non-target-specific options passed to host compiler.
* -foffload=<options>
  Tells GCC to build offload images for all targets specified in configure. 
  They will be built with non-target-specific options passed to host compiler
  plus <options>.
* -foffload=<targets>=<options>
  Tells GCC to build offload images for <targets>.
  They will be built with non-target-specific options passed to host compiler
  plus <options>.

Options specified by -foffload are appended to the end of option set, so in case
of option conflicts they have more priority.

2. -foffload-abi=[lp64|ilp32]
   This option is supposed to tell mkoffload (and offload compiler) which ABI is
used in streamed GIMPLE.  This option is desirable, because host and offload
compilers must have the same ABI.  The option is generated by the host compiler
automatically, it should not be specified by user.

Examples:
$ gcc -fopenmp -c -O2 test1.c
$ gcc -fopenmp -c -O1 -msse -foffload=-mavx test2.c
$ gcc -fopenmp -foffload="-O3 -v" test1.o test2.o

In this example the offload images will be built with the following options:
"-O2 -mavx -O3 -v" for targets specified in configure.

$ gcc -fopenmp -foffload=x86_64-intelmicemul-linux-gnu="-mavx2" \
  -foffload=nvptx-none -foffload="-O3" -O2 test.c

In this example 2 offload images will be built:
for MIC with "-O2 -mavx2 -O3" and for PTX with "-O2 -O3".

Bootstrapped and regtested on top of patch 5.  Is it OK for trunk?
kyukhin/gomp4-offload branch is updated correspondingly.

Thanks,
  -- Ilya


2014-10-11  Bernd Schmidt  <bernds@codesourcery.com>
	    Andrey Turetskiy  <andrey.turetskiy@intel.com>
	    Ilya Verbin  <ilya.verbin@intel.com>

gcc/
	* common.opt (foffload, foffload-abi): New options.
	* config/i386/i386.c (ix86_offload_options): New static function.
	(TARGET_OFFLOAD_OPTIONS): Define.
	* coretypes.h (enum offload_abi): New enum.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in (TARGET_OFFLOAD_OPTIONS): Document.
	* gcc.c (offload_targets): New static variable.
	(handle_foffload_option): New static function.
	(driver_handle_option): Handle OPT_foffload_.
	(driver::maybe_putenv_OFFLOAD_TARGETS): Set OFFLOAD_TARGET_NAMES
	according to offload_targets.
	* hooks.c (hook_charptr_void_null): New hook.
	* hooks.h (hook_charptr_void_null): Declare.
	* lto-opts.c: Include lto-section-names.h.
	(lto_write_options): Append options from target offload_options hook and
	store them to offload_lto section.  Do not store target-specific,
	driver and diagnostic options in offload_lto section.
	* lto-wrapper.c (merge_and_complain): Handle OPT_foffload_ and
	OPT_foffload_abi_.
	(append_compiler_options, append_linker_options)
	(append_offload_options): New static functions.
	(compile_offload_image): Add new arguments with options.
	Call append_compiler_options and append_offload_options.
	(compile_images_for_offload_targets): Add new arguments with options.
	(find_and_merge_options): New static function.
	(run_gcc): Outline options handling into the new functions:
	find_and_merge_options, append_compiler_options, append_linker_options.
	* opts.c (common_handle_option): Don't handle OPT_foffload_.
	* target.def (offload_options): New target hook.

---

diff --git a/gcc/common.opt b/gcc/common.opt
index b4f0ed4..37a5fd4 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1640,6 +1640,23 @@ fnon-call-exceptions
 Common Report Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions
 
+foffload=
+Common Driver Joined MissingArgError(options or targets missing after %qs)
+-foffload=<targets>=<options>  Specify offloading targets and options for them
+
+foffload-abi=
+Common Joined RejectNegative Enum(offload_abi) Var(flag_offload_abi) Init(OFFLOAD_ABI_UNSET)
+-foffload-abi=[lp64|ilp32]     Set the ABI to use in an offload compiler
+
+Enum
+Name(offload_abi) Type(enum offload_abi) UnknownError(unknown offload ABI %qs)
+
+EnumValue
+Enum(offload_abi) String(ilp32) Value(OFFLOAD_ABI_ILP32)
+
+EnumValue
+Enum(offload_abi) String(lp64) Value(OFFLOAD_ABI_LP64)
+
 fomit-frame-pointer
 Common Report Var(flag_omit_frame_pointer) Optimization
 When possible do not generate stack frames
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ed8fe2d..ec0c5d6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4301,6 +4301,15 @@ ix86_option_override (void)
   register_pass (&insert_vzeroupper_info);
 }
 
+/* Implement the TARGET_OFFLOAD_OPTIONS hook.  */
+static char *
+ix86_offload_options (void)
+{
+  if (TARGET_LP64)
+    return xstrdup ("-foffload-abi=lp64");
+  return xstrdup ("-foffload-abi=ilp32");
+}
+
 /* Update register usage after having seen the compiler flags.  */
 
 static void
@@ -47621,6 +47630,10 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
 #undef TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS
 #define TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS true
 
+#undef TARGET_OFFLOAD_OPTIONS
+#define TARGET_OFFLOAD_OPTIONS \
+  ix86_offload_options
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 #include "gt-i386.h"
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index c850ff4..7de374b 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -132,6 +132,13 @@ enum tls_model {
   TLS_MODEL_LOCAL_EXEC
 };
 
+/* Types of ABI for an offload compiler.  */
+enum offload_abi {
+  OFFLOAD_ABI_UNSET,
+  OFFLOAD_ABI_LP64,
+  OFFLOAD_ABI_ILP32
+};
+
 /* Types of unwind/exception handling info that can be generated.  */
 
 enum unwind_info_type
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 80da884..61fb994 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11201,6 +11201,15 @@ sections are available.  It is called once for each symbol that must be
 recorded in the offload function and variable table.
 @end deftypefn
 
+@deftypefn {Target Hook} {char *} TARGET_OFFLOAD_OPTIONS (void)
+Used when writing out the list of options into an LTO file.  It should
+translate any relevant target-specific options (such as the ABI in use)
+into one of the @option{-foffload} options that exist as a common interface
+to express such options.  It should return a string containing these options,
+separated by spaces, which the caller will free.
+
+@end deftypefn
+
 @defmac TARGET_SUPPORTS_WIDE_INT
 
 On older ports, large integers are stored in @code{CONST_DOUBLE} rtl
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 2b5d4f0..eefe5e3 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -8165,6 +8165,8 @@ and the associated definitions of those functions.
 
 @hook TARGET_RECORD_OFFLOAD_SYMBOL
 
+@hook TARGET_OFFLOAD_OPTIONS
+
 @defmac TARGET_SUPPORTS_WIDE_INT
 
 On older ports, large integers are stored in @code{CONST_DOUBLE} rtl
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 63c86e1..728b216 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -159,6 +159,10 @@ 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.  */
+
+static char *offload_targets = NULL;
+
 /* Nonzero if cross-compiling.
    When -b is used, the value comes from the `specs' file.  */
 
@@ -3362,6 +3366,92 @@ driver_wrong_lang_callback (const struct cl_decoded_option *decoded,
 static const char *spec_lang = 0;
 static int last_language_n_infiles;
 
+/* Parse -foffload option argument.  */
+
+static void
+handle_foffload_option (const char *arg)
+{
+  const char *c, *cur, *n, *next, *end;
+  char *target;
+
+  /* If option argument starts with '-' then no target is specified and we
+     do not need to parse it.  */
+  if (arg[0] == '-')
+    return;
+
+  end = strchrnul (arg, '=');
+  cur = arg;
+
+  while (cur < end)
+    {
+      next = strchrnul (cur, ',');
+      next = (next > end) ? end : next;
+
+      target = XNEWVEC (char, next - cur + 1);
+      strncpy (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)
+	{
+	  free (offload_targets);
+	  offload_targets = xstrdup ("");
+	  break;
+	}
+
+      /* Check that GCC is configured to support the offload target.  */
+      c = OFFLOAD_TARGETS;
+      while (c)
+	{
+	  n = strchrnul (c, ',');
+
+	  if (strlen (target) == (size_t) (n - c)
+	      && strncmp (target, c, n - c) == 0)
+	    break;
+
+	  c = *n ? n + 1 : NULL;
+	}
+
+      if (!c)
+	fatal_error ("GCC is not configured to support %s as offload target",
+		     target);
+
+      if (!offload_targets)
+	offload_targets = xstrdup (target);
+      else
+	{
+	  /* Check that the target hasn't already presented in the list.  */
+	  c = offload_targets;
+	  do
+	    {
+	      n = strchrnul (c, ':');
+
+	      if (strlen (target) == (size_t) (n - c)
+		  && strncmp (c, target, n - c) == 0)
+		break;
+
+	      c = n + 1;
+	    }
+	  while (*n);
+
+	  /* If duplicate is not found, append the target to the list.  */
+	  if (c > n)
+	    {
+	      offload_targets
+		= XRESIZEVEC (char, offload_targets,
+			      strlen (offload_targets) + strlen (target) + 2);
+	      if (strlen (offload_targets) != 0)
+		strcat (offload_targets, ":");
+	      strcat (offload_targets, target);
+	    }
+	}
+
+      cur = next + 1;
+      XDELETEVEC (target);
+    }
+}
+
 /* Handle a driver option; arguments and return value as for
    handle_option.  */
 
@@ -3738,6 +3828,10 @@ driver_handle_option (struct gcc_options *opts,
       flag_wpa = "";
       break;
 
+    case OPT_foffload_:
+      handle_foffload_option (arg);
+      break;
+
     default:
       /* Various driver options need no special processing at this
 	 point, having been handled in a prescan above or being
@@ -7240,14 +7334,22 @@ driver::maybe_putenv_COLLECT_LTO_WRAPPER () const
 void
 driver::maybe_putenv_OFFLOAD_TARGETS () const
 {
-  if (strlen (OFFLOAD_TARGETS) > 0)
+  const char *targets = offload_targets;
+
+  /* If no targets specified by -foffload, use all available targets.  */
+  if (!targets)
+    targets = OFFLOAD_TARGETS;
+
+  if (strlen (targets) > 0)
     {
       obstack_grow (&collect_obstack, "OFFLOAD_TARGET_NAMES=",
 		    sizeof ("OFFLOAD_TARGET_NAMES=") - 1);
-      obstack_grow (&collect_obstack, OFFLOAD_TARGETS,
-		    strlen (OFFLOAD_TARGETS) + 1);
+      obstack_grow (&collect_obstack, targets,
+		    strlen (targets) + 1);
       xputenv (XOBFINISH (&collect_obstack, char *));
     }
+
+  free (offload_targets);
 }
 
 /* Reject switches that no pass was interested in.  */
diff --git a/gcc/hooks.c b/gcc/hooks.c
index ce62dfd..d4c172a 100644
--- a/gcc/hooks.c
+++ b/gcc/hooks.c
@@ -374,13 +374,20 @@ hook_tree_tree_tree_tree_3rd_identity (tree a ATTRIBUTE_UNUSED,
   return c;
 }
 
-/* Generic hook that takes no arguments and returns a NULL string.  */
+/* Generic hook that takes no arguments and returns a NULL const string.  */
 const char *
 hook_constcharptr_void_null (void)
 {
   return NULL;
 }
 
+/* Generic hook that takes no arguments and returns a NULL string.  */
+char *
+hook_charptr_void_null (void)
+{
+  return NULL;
+}
+
 /* Generic hook that takes a tree and returns a NULL string.  */
 const char *
 hook_constcharptr_const_tree_null (const_tree t ATTRIBUTE_UNUSED)
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 76a6551..1142aa4 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -103,6 +103,7 @@ extern rtx hook_rtx_rtx_identity (rtx);
 extern rtx hook_rtx_rtx_null (rtx);
 extern rtx hook_rtx_tree_int_null (tree, int);
 
+extern char *hook_charptr_void_null (void);
 extern const char *hook_constcharptr_void_null (void);
 extern const char *hook_constcharptr_const_tree_null (const_tree);
 extern const char *hook_constcharptr_const_rtx_insn_null (const rtx_insn *);
diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index ceccb6f..f008e60 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "common/common-target.h"
 #include "diagnostic.h"
 #include "lto-streamer.h"
+#include "lto-section-names.h"
 #include "toplev.h"
 
 /* Append the option piece OPT to the COLLECT_GCC_OPTIONS string
@@ -146,6 +147,23 @@ lto_write_options (void)
     append_to_collect_gcc_options (&temporary_obstack, &first_p,
 			       "-fno-strict-overflow");
 
+  /* Append options from target hook and store them to offload_lto section.  */
+  if (strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) == 0)
+    {
+      char *offload_opts = targetm.offload_options ();
+      char *offload_ptr = offload_opts;
+      while (offload_ptr)
+       {
+	 char *next = strchr (offload_ptr, ' ');
+	 if (next)
+	   *next++ = '\0';
+	 append_to_collect_gcc_options (&temporary_obstack, &first_p,
+					offload_ptr);
+	 offload_ptr = next;
+       }
+      free (offload_opts);
+    }
+
   /* Output explicitly passed options.  */
   for (i = 1; i < save_decoded_options_count; ++i)
     {
@@ -169,15 +187,23 @@ lto_write_options (void)
       if (!(cl_options[option->opt_index].flags & (CL_COMMON|CL_TARGET|CL_LTO)))
 	continue;
 
+      /* Do not store target-specific options in offload_lto section.  */
+      if ((cl_options[option->opt_index].flags & CL_TARGET)
+	 && strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) == 0)
+       continue;
+
       /* Drop options created from the gcc driver that will be rejected
 	 when passed on to the driver again.  */
       if (cl_options[option->opt_index].cl_reject_driver)
 	continue;
 
       /* Also drop all options that are handled by the driver as well,
-         which includes things like -o and -v or -fhelp for example.
-	 We do not need those.  Also drop all diagnostic options.  */
-      if (cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
+	 which includes things like -o and -v or -fhelp for example.
+	 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))
+	  && (strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) != 0
+	      || option->opt_index != OPT_foffload_))
 	continue;
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index f5a508d..9e6250a 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -296,6 +296,17 @@ merge_and_complain (struct cl_decoded_option **decoded_options,
 			 " files", foption->orig_option_with_args_text);
 	  break;
 
+	case OPT_foffload_abi_:
+	  for (j = 0; j < *decoded_options_count; ++j)
+	    if ((*decoded_options)[j].opt_index == foption->opt_index)
+	      break;
+	    if (j == *decoded_options_count)
+	      append_option (decoded_options, decoded_options_count, foption);
+	    else if (foption->value != (*decoded_options)[j].value)
+	      fatal_error ("Option %s not used consistently in all LTO input"
+			   " files", foption->orig_option_with_args_text);
+	    break;
+
 	case OPT_O:
 	case OPT_Ofast:
 	case OPT_Og:
@@ -366,6 +377,10 @@ merge_and_complain (struct cl_decoded_option **decoded_options,
 	      (*decoded_options)[j].value = 1;
 	    }
 	  break;
+
+	case OPT_foffload_:
+	  append_option (decoded_options, decoded_options_count, foption);
+	  break;
 	}
     }
 }
@@ -427,6 +442,167 @@ parse_env_var (const char *str, char ***pvalues, const char *append)
   return num;
 }
 
+/* Append options OPTS from lto or offload_lto sections to ARGV_OBSTACK.  */
+
+static void
+append_compiler_options (obstack *argv_obstack, struct cl_decoded_option *opts,
+			 unsigned int count)
+{
+  /* Append compiler driver arguments as far as they were merged.  */
+  for (unsigned int j = 1; j < count; ++j)
+    {
+      struct cl_decoded_option *option = &opts[j];
+
+      /* File options have been properly filtered by lto-opts.c.  */
+      switch (option->opt_index)
+	{
+	/* Drop arguments that we want to take from the link line.  */
+	case OPT_flto_:
+	case OPT_flto:
+	case OPT_flto_partition_:
+	  continue;
+
+	default:
+	  break;
+	}
+
+      /* For now do what the original LTO option code was doing - pass
+	 on any CL_TARGET flag and a few selected others.  */
+      switch (option->opt_index)
+	{
+	case OPT_fPIC:
+	case OPT_fpic:
+	case OPT_fPIE:
+	case OPT_fpie:
+	case OPT_fcommon:
+	case OPT_fexceptions:
+	case OPT_fnon_call_exceptions:
+	case OPT_fgnu_tm:
+	case OPT_freg_struct_return:
+	case OPT_fpcc_struct_return:
+	case OPT_fshort_double:
+	case OPT_ffp_contract_:
+	case OPT_fmath_errno:
+	case OPT_fsigned_zeros:
+	case OPT_ftrapping_math:
+	case OPT_fwrapv:
+	case OPT_ftrapv:
+	case OPT_fstrict_overflow:
+	case OPT_foffload_abi_:
+	case OPT_O:
+	case OPT_Ofast:
+	case OPT_Og:
+	case OPT_Os:
+	  break;
+
+	default:
+	  if (!(cl_options[option->opt_index].flags & CL_TARGET))
+	    continue;
+	}
+
+      /* Pass the option on.  */
+      for (unsigned int i = 0; i < option->canonical_option_num_elements; ++i)
+	obstack_ptr_grow (argv_obstack, option->canonical_option[i]);
+    }
+}
+
+/* Append linker options OPTS to ARGV_OBSTACK.  */
+
+static void
+append_linker_options (obstack *argv_obstack, struct cl_decoded_option *opts,
+		       unsigned int count)
+{
+  /* Append linker driver arguments.  Compiler options from the linker
+     driver arguments will override / merge with those from the compiler.  */
+  for (unsigned int j = 1; j < count; ++j)
+    {
+      struct cl_decoded_option *option = &opts[j];
+
+      /* Do not pass on frontend specific flags not suitable for lto.  */
+      if (!(cl_options[option->opt_index].flags
+	    & (CL_COMMON|CL_TARGET|CL_DRIVER|CL_LTO)))
+	continue;
+
+      switch (option->opt_index)
+	{
+	case OPT_o:
+	case OPT_flto_:
+	case OPT_flto:
+	  /* We've handled these LTO options, do not pass them on.  */
+	  continue;
+
+	case OPT_freg_struct_return:
+	case OPT_fpcc_struct_return:
+	case OPT_fshort_double:
+	  /* Ignore these, they are determined by the input files.
+	     ???  We fail to diagnose a possible mismatch here.  */
+	  continue;
+
+	default:
+	  break;
+	}
+
+      /* Pass the option on.  */
+      for (unsigned int i = 0; i < option->canonical_option_num_elements; ++i)
+	obstack_ptr_grow (argv_obstack, option->canonical_option[i]);
+    }
+}
+
+/* Extract options for TARGET offload compiler from OPTIONS and append
+   them to ARGV_OBSTACK.  */
+
+static void
+append_offload_options (obstack *argv_obstack, const char *target,
+			struct cl_decoded_option *options,
+			unsigned int options_count)
+{
+  for (unsigned i = 0; i < options_count; i++)
+    {
+      const char *cur, *next, *opts;
+      char **argv;
+      unsigned argc;
+      struct cl_decoded_option *option = &options[i];
+
+      if (option->opt_index != OPT_foffload_)
+	continue;
+
+      /* If option argument starts with '-' then no target is specified.  That
+	 means offload options are specified for all targets, so we need to
+	 append them.  */
+      if (option->arg[0] == '-')
+	opts = option->arg;
+      else
+	{
+	  opts = strchr (option->arg, '=');
+	  if (!opts)
+	    continue;
+
+	  cur = option->arg;
+
+	  while (cur < opts)
+	    {
+	      next = strchrnul (cur, ',');
+	      next = (next > opts) ? opts : next;
+
+	      if (strlen (target) == (size_t) (next - cur)
+		  && strncmp (target, cur, next - cur) == 0)
+		break;
+
+	      cur = next + 1;
+	    }
+
+	  if (cur >= opts)
+	    continue;
+
+	  opts++;
+	}
+
+      argv = buildargv (opts);
+      for (argc = 0; argv[argc]; argc++)
+	obstack_ptr_grow (argv_obstack, argv[argc]);
+    }
+}
+
 /* Check whether NAME can be accessed in MODE.  This is like access,
    except that it never considers directories to be executable.  */
 
@@ -450,7 +626,11 @@ access_check (const char *name, int mode)
 
 static char *
 compile_offload_image (const char *target, const char *compiler_path,
-		       unsigned in_argc, char *in_argv[])
+		       unsigned in_argc, char *in_argv[],
+		       struct cl_decoded_option *compiler_opts,
+		       unsigned int compiler_opt_count,
+		       struct cl_decoded_option *linker_opts,
+		       unsigned int linker_opt_count)
 {
   char *filename = NULL;
   char **argv;
@@ -482,10 +662,22 @@ compile_offload_image (const char *target, const char *compiler_path,
       obstack_ptr_grow (&argv_obstack, "-o");
       obstack_ptr_grow (&argv_obstack, filename);
 
+      /* Append names of input object files.  */
       for (unsigned i = 1; i < in_argc; i++)
 	obstack_ptr_grow (&argv_obstack, in_argv[i]);
-      obstack_ptr_grow (&argv_obstack, NULL);
 
+      /* Append options from offload_lto sections.  */
+      append_compiler_options (&argv_obstack, compiler_opts,
+			       compiler_opt_count);
+
+      /* Append options specified by -foffload last.  In case of conflicting
+	 options we expect offload compiler to choose the latest.  */
+      append_offload_options (&argv_obstack, target, compiler_opts,
+			      compiler_opt_count);
+      append_offload_options (&argv_obstack, target, linker_opts,
+			      linker_opt_count);
+
+      obstack_ptr_grow (&argv_obstack, NULL);
       argv = XOBFINISH (&argv_obstack, char **);
       fork_execute (argv[0], argv, true);
       obstack_free (&argv_obstack, NULL);
@@ -502,7 +694,11 @@ compile_offload_image (const char *target, const char *compiler_path,
    target sections, we pass them all to target compilers.  */
 
 static void
-compile_images_for_offload_targets (unsigned in_argc, char *in_argv[])
+compile_images_for_offload_targets (unsigned in_argc, char *in_argv[],
+				    struct cl_decoded_option *compiler_opts,
+				    unsigned int compiler_opt_count,
+				    struct cl_decoded_option *linker_opts,
+				    unsigned int linker_opt_count)
 {
   char **names = NULL;
   const char *target_names = getenv (OFFLOAD_TARGET_NAMES_ENV);
@@ -519,8 +715,10 @@ compile_images_for_offload_targets (unsigned in_argc, char *in_argv[])
   offload_names = XCNEWVEC (char *, num_targets + 1);
   for (unsigned i = 0; i < num_targets; i++)
     {
-      offload_names[i] = compile_offload_image (names[i], compiler_path,
-						in_argc, in_argv);
+      offload_names[i]
+	= compile_offload_image (names[i], compiler_path, in_argc, in_argv,
+				 compiler_opts, compiler_opt_count,
+				 linker_opts, linker_opt_count);
       if (!offload_names[i])
 	fatal_error ("problem with building target image for %s\n", names[i]);
     }
@@ -587,6 +785,74 @@ find_offloadbeginend (void)
   free_array_of_ptrs ((void **) paths, n_paths);
 }
 
+/* A subroutine of run_gcc.  Examine the open file FD for lto sections with
+   name prefix PREFIX, at FILE_OFFSET, and store any options we find in OPTS
+   and OPT_COUNT.  Return true if we found a matchingn section, false
+   otherwise.  COLLECT_GCC holds the value of the environment variable with
+   the same name.  */
+
+static bool
+find_and_merge_options (int fd, off_t file_offset, const char *prefix,
+			struct cl_decoded_option **opts,
+			unsigned int *opt_count, const char *collect_gcc)
+{
+  off_t offset, length;
+  char *data;
+  char *fopts;
+  const char *errmsg;
+  int err;
+  struct cl_decoded_option *fdecoded_options = *opts;
+  unsigned int fdecoded_options_count = *opt_count;
+
+  simple_object_read *sobj;
+  sobj = simple_object_start_read (fd, file_offset, "__GNU_LTO",
+				   &errmsg, &err);
+  if (!sobj)
+    return false;
+
+  char *secname = XALLOCAVEC (char, strlen (prefix) + sizeof (".opts"));
+  strcpy (secname, prefix);
+  strcat (secname, ".opts");
+  if (!simple_object_find_section (sobj, secname, &offset, &length,
+				   &errmsg, &err))
+    {
+      simple_object_release_read (sobj);
+      return false;
+    }
+
+  lseek (fd, file_offset + offset, SEEK_SET);
+  data = (char *)xmalloc (length);
+  read (fd, data, length);
+  fopts = data;
+  do
+    {
+      struct cl_decoded_option *f2decoded_options;
+      unsigned int f2decoded_options_count;
+      get_options_from_collect_gcc_options (collect_gcc,
+					    fopts, CL_LANG_ALL,
+					    &f2decoded_options,
+					    &f2decoded_options_count);
+      if (!fdecoded_options)
+       {
+	 fdecoded_options = f2decoded_options;
+	 fdecoded_options_count = f2decoded_options_count;
+       }
+      else
+	merge_and_complain (&fdecoded_options,
+			    &fdecoded_options_count,
+			    f2decoded_options, f2decoded_options_count);
+
+      fopts += strlen (fopts) + 1;
+    }
+  while (fopts - data < length);
+
+  free (data);
+  simple_object_release_read (sobj);
+  *opts = fdecoded_options;
+  *opt_count = fdecoded_options_count;
+  return true;
+}
+
 /* Execute gcc. ARGC is the number of arguments. ARGV contains the arguments. */
 
 static void
@@ -602,7 +868,9 @@ run_gcc (unsigned argc, char *argv[])
   int jobserver = 0;
   bool no_partition = false;
   struct cl_decoded_option *fdecoded_options = NULL;
+  struct cl_decoded_option *offload_fdecoded_options = NULL;
   unsigned int fdecoded_options_count = 0;
+  unsigned int offload_fdecoded_options_count = 0;
   struct cl_decoded_option *decoded_options;
   unsigned int decoded_options_count;
   struct obstack argv_obstack;
@@ -625,18 +893,13 @@ run_gcc (unsigned argc, char *argv[])
   /* Look at saved options in the IL files.  */
   for (i = 1; i < argc; ++i)
     {
-      char *data, *p;
-      char *fopts;
+      char *p;
       int fd;
-      const char *errmsg;
-      int err;
-      off_t file_offset = 0, offset, length;
+      off_t file_offset = 0;
       long loffset;
-      simple_object_read *sobj;
       int consumed;
-      struct cl_decoded_option *f2decoded_options;
-      unsigned int f2decoded_options_count;
       char *filename = argv[i];
+
       if ((p = strrchr (argv[i], '@'))
 	  && p != argv[i] 
 	  && sscanf (p, "@%li%n", &loffset, &consumed) >= 1
@@ -650,50 +913,15 @@ run_gcc (unsigned argc, char *argv[])
       fd = open (argv[i], O_RDONLY);
       if (fd == -1)
 	continue;
-      sobj = simple_object_start_read (fd, file_offset, "__GNU_LTO", 
-	  			       &errmsg, &err);
-      if (!sobj)
-	{
-	  close (fd);
-	  continue;
-	}
-      if (simple_object_find_section (sobj, OFFLOAD_SECTION_NAME_PREFIX ".opts",
-				      &offset, &length, &errmsg, &err))
-	have_offload = true;
-      if (!simple_object_find_section (sobj, LTO_SECTION_NAME_PREFIX "." "opts",
-				       &offset, &length, &errmsg, &err))
-	{
-	  simple_object_release_read (sobj);
-	  close (fd);
-	  continue;
-	}
-      have_lto = true;
-      lseek (fd, file_offset + offset, SEEK_SET);
-      data = (char *)xmalloc (length);
-      read (fd, data, length);
-      fopts = data;
-      do
-	{
-	  get_options_from_collect_gcc_options (collect_gcc,
-						fopts, CL_LANG_ALL,
-						&f2decoded_options,
-						&f2decoded_options_count);
-	  if (!fdecoded_options)
-	    {
-	      fdecoded_options = f2decoded_options;
-	      fdecoded_options_count = f2decoded_options_count;
-	    }
-	  else
-	    merge_and_complain (&fdecoded_options,
-				&fdecoded_options_count,
-				f2decoded_options, f2decoded_options_count);
 
-	  fopts += strlen (fopts) + 1;
-	}
-      while (fopts - data < length);
-
-      free (data);
-      simple_object_release_read (sobj);
+      have_lto
+	= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
+				  &fdecoded_options, &fdecoded_options_count,
+				  collect_gcc);
+      have_offload
+	= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
+				  &offload_fdecoded_options,
+				  &offload_fdecoded_options_count, collect_gcc);
       close (fd);
     }
 
@@ -703,79 +931,19 @@ run_gcc (unsigned argc, char *argv[])
   obstack_ptr_grow (&argv_obstack, "-xlto");
   obstack_ptr_grow (&argv_obstack, "-c");
 
-  /* Append compiler driver arguments as far as they were merged.  */
-  for (j = 1; j < fdecoded_options_count; ++j)
-    {
-      struct cl_decoded_option *option = &fdecoded_options[j];
-
-      /* File options have been properly filtered by lto-opts.c.  */
-      switch (option->opt_index)
-	{
-	  /* Drop arguments that we want to take from the link line.  */
-	  case OPT_flto_:
-	  case OPT_flto:
-	  case OPT_flto_partition_:
-	      continue;
-
-	  default:
-	      break;
-	}
-
-      /* For now do what the original LTO option code was doing - pass
-	 on any CL_TARGET flag and a few selected others.  */
-      switch (option->opt_index)
-	{
-	case OPT_fPIC:
-	case OPT_fpic:
-	case OPT_fPIE:
-	case OPT_fpie:
-	case OPT_fcommon:
-	case OPT_fexceptions:
-	case OPT_fnon_call_exceptions:
-	case OPT_fgnu_tm:
-	case OPT_freg_struct_return:
-	case OPT_fpcc_struct_return:
-	case OPT_fshort_double:
-	case OPT_ffp_contract_:
-	case OPT_fmath_errno:
-	case OPT_fsigned_zeros:
-	case OPT_ftrapping_math:
-	case OPT_fwrapv:
-	case OPT_ftrapv:
-	case OPT_fstrict_overflow:
-	case OPT_O:
-	case OPT_Ofast:
-	case OPT_Og:
-	case OPT_Os:
-	  break;
-
-	default:
-	  if (!(cl_options[option->opt_index].flags & CL_TARGET))
-	    continue;
-	}
-
-      /* Pass the option on.  */
-      for (i = 0; i < option->canonical_option_num_elements; ++i)
-	obstack_ptr_grow (&argv_obstack, option->canonical_option[i]);
-    }
+  append_compiler_options (&argv_obstack, fdecoded_options,
+			   fdecoded_options_count);
+  append_linker_options (&argv_obstack, decoded_options, decoded_options_count);
 
-  /* Append linker driver arguments.  Compiler options from the linker
-     driver arguments will override / merge with those from the compiler.  */
+  /* Scan linker driver arguments for things that are of relevance to us.  */
   for (j = 1; j < decoded_options_count; ++j)
     {
       struct cl_decoded_option *option = &decoded_options[j];
-
-      /* Do not pass on frontend specific flags not suitable for lto.  */
-      if (!(cl_options[option->opt_index].flags
-	    & (CL_COMMON|CL_TARGET|CL_DRIVER|CL_LTO)))
-	continue;
-
       switch (option->opt_index)
 	{
 	case OPT_o:
 	  linker_output = option->arg;
-	  /* We generate new intermediate output, drop this arg.  */
-	  continue;
+	  break;
 
 	case OPT_save_temps:
 	  save_temps = 1;
@@ -806,23 +974,11 @@ run_gcc (unsigned argc, char *argv[])
 
 	case OPT_flto:
 	  lto_mode = LTO_MODE_WHOPR;
-	  /* We've handled these LTO options, do not pass them on.  */
-	  continue;
-
-	case OPT_freg_struct_return:
-	case OPT_fpcc_struct_return:
-	case OPT_fshort_double:
-	  /* Ignore these, they are determined by the input files.
-	     ???  We fail to diagnose a possible mismatch here.  */
-	  continue;
+	  break;
 
 	default:
 	  break;
 	}
-
-      /* Pass the option on.  */
-      for (i = 0; i < option->canonical_option_num_elements; ++i)
-	obstack_ptr_grow (&argv_obstack, option->canonical_option[i]);
     }
 
   if (no_partition)
@@ -864,7 +1020,10 @@ run_gcc (unsigned argc, char *argv[])
 
   if (have_offload)
     {
-      compile_images_for_offload_targets (argc, argv);
+      compile_images_for_offload_targets (argc, argv, offload_fdecoded_options,
+					  offload_fdecoded_options_count,
+					  decoded_options,
+					  decoded_options_count);
       if (offload_names)
 	{
 	  find_offloadbeginend ();
diff --git a/gcc/opts.c b/gcc/opts.c
index 5cb5a39..8c53356 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1726,6 +1726,10 @@ common_handle_option (struct gcc_options *opts,
       /* Deferred.  */
       break;
 
+    case OPT_foffload_:
+      /* Deferred.  */
+      break;
+
     case OPT_fpack_struct_:
       if (value <= 0 || (value & (value - 1)) || value > 16)
 	error_at (loc,
diff --git a/gcc/target.def b/gcc/target.def
index aa5a680..c4b28a2 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -1783,6 +1783,15 @@ actions then, you should have @code{TARGET_OPTION_OVERRIDE} call\n\
  void, (void),
  hook_void_void)
 
+DEFHOOK
+(offload_options,
+ "Used when writing out the list of options into an LTO file.  It should\n\
+translate any relevant target-specific options (such as the ABI in use)\n\
+into one of the @option{-foffload} options that exist as a common interface\n\
+to express such options.  It should return a string containing these options,\n\
+separated by spaces, which the caller will free.\n",
+char *, (void), hook_charptr_void_null)
+
 DEFHOOK_UNDOC
 (eh_return_filter_mode,
  "Return machine mode for filter value.",
-- 
1.7.1

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

[Jakub Jelinek]

On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> This is the last common infrastructure patch in the series.
> (Next patches will contain tests for libgomp testsuite and MIC specific things)
> 
> It introduces 2 new options:
> 1. -foffload=<targets>=<options>
>    By default, GCC will build offload images for all offload targets specified
> in configure, with non-target-specific options passed to host compiler.
> This option is used to control offload targets and options for them.
> 
> It can be used in a few ways:
> * -foffload=disable
>   Tells GCC to disable offload support.
>   OpenMP target regions will be run in host fallback mode.
> * -foffload=<targets>
>   Tells GCC to build offload images for <targets>.
>   They will be built with non-target-specific options passed to host compiler.
> * -foffload=<options>
>   Tells GCC to build offload images for all targets specified in configure. 
>   They will be built with non-target-specific options passed to host compiler
>   plus <options>.
> * -foffload=<targets>=<options>
>   Tells GCC to build offload images for <targets>.
>   They will be built with non-target-specific options passed to host compiler
>   plus <options>.
> 
> Options specified by -foffload are appended to the end of option set, so in case
> of option conflicts they have more priority.

This looks good to me.

> 2. -foffload-abi=[lp64|ilp32]
>    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> used in streamed GIMPLE.  This option is desirable, because host and offload
> compilers must have the same ABI.  The option is generated by the host compiler
> automatically, it should not be specified by user.

But I'd like to understand why is this one needed.
Why should the compilers care?  Aggregates layout and alignment of
integral/floating types must match between host and offload compilers, sure,
but isn't that something streamed already in the LTO bytecode?
Or is LTO streamer not streaming some types like long_type_node?
I'd expect if host and offload compiler disagree on long type size that
you'd just use a different integral type with the same size as long on the
host.
Different sized pointers are of course a bigger problem, but can't you just
error out on that during reading of the LTO, or even handle it (just use
some integral type for when is the pointer stored in memory, and just
convert to pointer after reads from memory, and convert back before storing
to memory).  Erroring out during LTO streaming in sounds just fine to me
though.

	Jakub

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

[Ilya Verbin]

On 13 Oct 12:19, Jakub Jelinek wrote:
> But I'd like to understand why is this one needed.
> Why should the compilers care?  Aggregates layout and alignment of
> integral/floating types must match between host and offload compilers, sure,
> but isn't that something streamed already in the LTO bytecode?
> Or is LTO streamer not streaming some types like long_type_node?
> I'd expect if host and offload compiler disagree on long type size that
> you'd just use a different integral type with the same size as long on the
> host.
> Different sized pointers are of course a bigger problem, but can't you just
> error out on that during reading of the LTO, or even handle it (just use
> some integral type for when is the pointer stored in memory, and just
> convert to pointer after reads from memory, and convert back before storing
> to memory).  Erroring out during LTO streaming in sounds just fine to me
> though.

Actually this option was developed by Bernd, so I think PTX team is going to use
it somehow.  In MIC's case we're planning just to check in mkoffload that host
and target compiler's ABI are the same.  Without this check we will crash in LTO
streamer with ICE, so I'd like to issue an error message, rather than crashing.

  -- Ilya

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

[Bernd Schmidt]

On 10/13/2014 12:33 PM, Ilya Verbin wrote:
> On 13 Oct 12:19, Jakub Jelinek wrote:
>> But I'd like to understand why is this one needed.
>> Why should the compilers care?  Aggregates layout and alignment of
>> integral/floating types must match between host and offload compilers, sure,
>> but isn't that something streamed already in the LTO bytecode?
>> Or is LTO streamer not streaming some types like long_type_node?

It isn't, see the preload_common_nodes code. Also, the backend needs to 
choose the right Pmode (and in the case of ptx, emit a directive about 
address sizes).


Bernd

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

[Richard Biener]

On Mon, 13 Oct 2014, Bernd Schmidt wrote:

> On 10/13/2014 12:33 PM, Ilya Verbin wrote:
> > On 13 Oct 12:19, Jakub Jelinek wrote:
> > > But I'd like to understand why is this one needed.
> > > Why should the compilers care?  Aggregates layout and alignment of
> > > integral/floating types must match between host and offload compilers,
> > > sure,
> > > but isn't that something streamed already in the LTO bytecode?
> > > Or is LTO streamer not streaming some types like long_type_node?
> 
> It isn't, see the preload_common_nodes code.

Something I'd like to get rid of at some point (but it's not 100%
easy as backends for example compare va_list_type_node by pointer).

> Also, the backend needs to choose
> the right Pmode (and in the case of ptx, emit a directive about address
> sizes).

Surely that will only be one problem with going the LTO way to handle
the offloading ;)

Richard.

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

[Bernd Schmidt]

On 10/14/2014 09:25 AM, Richard Biener wrote:
> On Mon, 13 Oct 2014, Bernd Schmidt wrote:
>
>> On 10/13/2014 12:33 PM, Ilya Verbin wrote:
>>> On 13 Oct 12:19, Jakub Jelinek wrote:
>>>> But I'd like to understand why is this one needed.
>>>> Why should the compilers care?  Aggregates layout and alignment of
>>>> integral/floating types must match between host and offload compilers,
>>>> sure,
>>>> but isn't that something streamed already in the LTO bytecode?
>>>> Or is LTO streamer not streaming some types like long_type_node?
>>
>> It isn't, see the preload_common_nodes code.
>
> Something I'd like to get rid of at some point (but it's not 100%
> easy as backends for example compare va_list_type_node by pointer).

Hmm, this is unfortunate - I was about to submit a patch not to stream 
that one since it can differ between host and offload target.

I see one such comparison in i386.c - any others you are aware of? 
Should it be sufficient to just compare the TYPE_MAIN_VARIANT instead?

>> Also, the backend needs to choose
>> the right Pmode (and in the case of ptx, emit a directive about address
>> sizes).
>
> Surely that will only be one problem with going the LTO way to handle
> the offloading ;)

Another problem I mentioned above, beyond that I have a patch to use the 
$host-modes.def file to define machine modes - and that's essentially it.

I'll be submitting these additional offloading patches for the case of 
different host and target once Ilya has committed the others.


Bernd

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

[Richard Biener]

On Tue, Oct 14, 2014 at 1:49 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 10/14/2014 09:25 AM, Richard Biener wrote:
>>
>> On Mon, 13 Oct 2014, Bernd Schmidt wrote:
>>
>>> On 10/13/2014 12:33 PM, Ilya Verbin wrote:
>>>>
>>>> On 13 Oct 12:19, Jakub Jelinek wrote:
>>>>>
>>>>> But I'd like to understand why is this one needed.
>>>>> Why should the compilers care?  Aggregates layout and alignment of
>>>>> integral/floating types must match between host and offload compilers,
>>>>> sure,
>>>>> but isn't that something streamed already in the LTO bytecode?
>>>>> Or is LTO streamer not streaming some types like long_type_node?
>>>
>>>
>>> It isn't, see the preload_common_nodes code.
>>
>>
>> Something I'd like to get rid of at some point (but it's not 100%
>> easy as backends for example compare va_list_type_node by pointer).
>
>
> Hmm, this is unfortunate - I was about to submit a patch not to stream that
> one since it can differ between host and offload target.
>
> I see one such comparison in i386.c - any others you are aware of?

No, x86 is the only target I ever tested.

> Should it
> be sufficient to just compare the TYPE_MAIN_VARIANT instead?

Well, they will not even be variants of each other.  ISTR complications
especially on x86 because of the alternate ms_abi va_list.

I'd say that we eventually should have a type flag that says
"this is a va-list type".  If we really need to know that - because
I don't understand why we need to do this - the context should
tell us exactly whether we deal with a va_list object or not ...

Richard.

>>> Also, the backend needs to choose
>>> the right Pmode (and in the case of ptx, emit a directive about address
>>> sizes).
>>
>>
>> Surely that will only be one problem with going the LTO way to handle
>> the offloading ;)
>
>
> Another problem I mentioned above, beyond that I have a patch to use the
> $host-modes.def file to define machine modes - and that's essentially it.
>
> I'll be submitting these additional offloading patches for the case of
> different host and target once Ilya has committed the others.
>
>
> Bernd
>

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

[Ilya Verbin]

On 13 Oct 12:19, Jakub Jelinek wrote:
> On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> > It introduces 2 new options:
> > 1. -foffload=<targets>=<options>
> >    By default, GCC will build offload images for all offload targets specified
> > in configure, with non-target-specific options passed to host compiler.
> > This option is used to control offload targets and options for them.
> > 
> > It can be used in a few ways:
> > * -foffload=disable
> >   Tells GCC to disable offload support.
> >   OpenMP target regions will be run in host fallback mode.
> > * -foffload=<targets>
> >   Tells GCC to build offload images for <targets>.
> >   They will be built with non-target-specific options passed to host compiler.
> > * -foffload=<options>
> >   Tells GCC to build offload images for all targets specified in configure. 
> >   They will be built with non-target-specific options passed to host compiler
> >   plus <options>.
> > * -foffload=<targets>=<options>
> >   Tells GCC to build offload images for <targets>.
> >   They will be built with non-target-specific options passed to host compiler
> >   plus <options>.
> > 
> > Options specified by -foffload are appended to the end of option set, so in case
> > of option conflicts they have more priority.
> 
> This looks good to me.
> 
> > 2. -foffload-abi=[lp64|ilp32]
> >    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> > used in streamed GIMPLE.  This option is desirable, because host and offload
> > compilers must have the same ABI.  The option is generated by the host compiler
> > automatically, it should not be specified by user.
> 
> But I'd like to understand why is this one needed.
> Why should the compilers care?  Aggregates layout and alignment of
> integral/floating types must match between host and offload compilers, sure,
> but isn't that something streamed already in the LTO bytecode?
> Or is LTO streamer not streaming some types like long_type_node?
> I'd expect if host and offload compiler disagree on long type size that
> you'd just use a different integral type with the same size as long on the
> host.
> Different sized pointers are of course a bigger problem, but can't you just
> error out on that during reading of the LTO, or even handle it (just use
> some integral type for when is the pointer stored in memory, and just
> convert to pointer after reads from memory, and convert back before storing
> to memory).  Erroring out during LTO streaming in sounds just fine to me
> though.

Here is an additional check.  Is the whole 'option handling' patch OK?

Thanks,
  -- Ilya


diff --git a/gcc/opts.c b/gcc/opts.c
index 9b2e1af..d1a626c 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1732,6 +1732,13 @@ common_handle_option (struct gcc_options *opts,
       /* Deferred.  */
       break;
 
+#ifndef ACCEL_COMPILER
+    case OPT_foffload_abi_:
+      error_at (loc, "-foffload-abi option can be specified only for "
+		"offload compiler");
+      break;
+#endif
+
     case OPT_fpack_struct_:
       if (value <= 0 || (value & (value - 1)) || value > 16)
 	error_at (loc,

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

[Jakub Jelinek]

On Fri, Oct 24, 2014 at 10:29:50PM +0400, Ilya Verbin wrote:
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 9b2e1af..d1a626c 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1732,6 +1732,13 @@ common_handle_option (struct gcc_options *opts,
>        /* Deferred.  */
>        break;
>  
> +#ifndef ACCEL_COMPILER

ifndef ?  I would have expected ifdef.

> +    case OPT_foffload_abi_:
> +      error_at (loc, "-foffload-abi option can be specified only for "
> +		"offload compiler");
> +      break;
> +#endif
> +
>      case OPT_fpack_struct_:
>        if (value <= 0 || (value & (value - 1)) || value > 16)
>  	error_at (loc,

	Jakub

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

[Bernd Schmidt]

On 10/15/2014 03:52 PM, Richard Biener wrote:

> I'd say that we eventually should have a type flag that says
> "this is a va-list type".  If we really need to know that - because
> I don't understand why we need to do this - the context should
> tell us exactly whether we deal with a va_list object or not ...

How about just an internal attribute?


Bernd

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

[Ilya Verbin]

On 24 Oct 20:33, Jakub Jelinek wrote:
> On Fri, Oct 24, 2014 at 10:29:50PM +0400, Ilya Verbin wrote:
> > diff --git a/gcc/opts.c b/gcc/opts.c
> > index 9b2e1af..d1a626c 100644
> > --- a/gcc/opts.c
> > +++ b/gcc/opts.c
> > @@ -1732,6 +1732,13 @@ common_handle_option (struct gcc_options *opts,
> >        /* Deferred.  */
> >        break;
> >  
> > +#ifndef ACCEL_COMPILER
> 
> ifndef ?  I would have expected ifdef.
> 
> > +    case OPT_foffload_abi_:
> > +      error_at (loc, "-foffload-abi option can be specified only for "
> > +		"offload compiler");
> > +      break;
> > +#endif
> > +
> >      case OPT_fpack_struct_:
> >        if (value <= 0 || (value & (value - 1)) || value > 16)
> >  	error_at (loc,

-foffload-abi option is intended only for the offload compiler, but we can't put
the option under ifdef in common.opt, therefore I added error_at under ifndef.

  -- Ilya

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

[Ilya Verbin]

On 11 Oct 18:49, Ilya Verbin wrote:
> 	(run_gcc): Outline options handling into the new functions:
> 	find_and_merge_options, append_compiler_options, append_linker_options.
> ...
> @@ -625,18 +893,13 @@ run_gcc (unsigned argc, char *argv[])
>    /* Look at saved options in the IL files.  */
>    for (i = 1; i < argc; ++i)
>      {
> ...
> +      have_lto
> +	= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
> +				  &fdecoded_options, &fdecoded_options_count,
> +				  collect_gcc);
> +      have_offload
> +	= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
> +				  &offload_fdecoded_options,
> +				  &offload_fdecoded_options_count, collect_gcc);
>        close (fd);
>      }

I found a bug here, have_{lto,offload} must be set if at least one file contains
lto/offload sections, but currently they are overwritten by the last file.
Fix is bootstrapped and regtested on x86_64-linux.  OK for trunk?


gcc/
	* lto-wrapper.c (run_gcc): Set have_lto and have_offload if at least one
	file contains sections with LTO and offload IR, respectively.


diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 9a540b4..0f69457 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -921,13 +921,14 @@ run_gcc (unsigned argc, char *argv[])
 	continue;
 
       have_lto
-	= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
-				  &fdecoded_options, &fdecoded_options_count,
-				  collect_gcc);
+	|= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
+				   &fdecoded_options, &fdecoded_options_count,
+				   collect_gcc);
       have_offload
-	= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
-				  &offload_fdecoded_options,
-				  &offload_fdecoded_options_count, collect_gcc);
+	|= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
+				   &offload_fdecoded_options,
+				   &offload_fdecoded_options_count,
+				   collect_gcc);
       close (fd);
     }
 

  -- Ilya

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

[Richard Biener]

On Fri, 28 Nov 2014, Ilya Verbin wrote:

> On 11 Oct 18:49, Ilya Verbin wrote:
> > 	(run_gcc): Outline options handling into the new functions:
> > 	find_and_merge_options, append_compiler_options, append_linker_options.
> > ...
> > @@ -625,18 +893,13 @@ run_gcc (unsigned argc, char *argv[])
> >    /* Look at saved options in the IL files.  */
> >    for (i = 1; i < argc; ++i)
> >      {
> > ...
> > +      have_lto
> > +	= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
> > +				  &fdecoded_options, &fdecoded_options_count,
> > +				  collect_gcc);
> > +      have_offload
> > +	= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
> > +				  &offload_fdecoded_options,
> > +				  &offload_fdecoded_options_count, collect_gcc);
> >        close (fd);
> >      }
> 
> I found a bug here, have_{lto,offload} must be set if at least one file contains
> lto/offload sections, but currently they are overwritten by the last file.
> Fix is bootstrapped and regtested on x86_64-linux.  OK for trunk?

Ok.

Thanks,
Richard.

> 
> gcc/
> 	* lto-wrapper.c (run_gcc): Set have_lto and have_offload if at least one
> 	file contains sections with LTO and offload IR, respectively.
> 
> 
> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index 9a540b4..0f69457 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -921,13 +921,14 @@ run_gcc (unsigned argc, char *argv[])
>  	continue;
>  
>        have_lto
> -	= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
> -				  &fdecoded_options, &fdecoded_options_count,
> -				  collect_gcc);
> +	|= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
> +				   &fdecoded_options, &fdecoded_options_count,
> +				   collect_gcc);
>        have_offload
> -	= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
> -				  &offload_fdecoded_options,
> -				  &offload_fdecoded_options_count, collect_gcc);
> +	|= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
> +				   &offload_fdecoded_options,
> +				   &offload_fdecoded_options_count,
> +				   collect_gcc);
>        close (fd);
>      }
>  
> 
>   -- Ilya
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany

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

[Ilya Verbin]

Hi,

On 28 Nov 09:36, Richard Biener wrote:
> On Fri, 28 Nov 2014, Ilya Verbin wrote:
> > I found a bug here, have_{lto,offload} must be set if at least one file contains
> > lto/offload sections, but currently they are overwritten by the last file.
> > Fix is bootstrapped and regtested on x86_64-linux.  OK for trunk?
> Ok.

Unfortunately, this fix was not general enough.
There might be cases when mixed object files get into lto-wrapper, ie some of
them contain only LTO sections, some contain only offload sections, and some
contain both.  But when lto-wrapper will pass all these files to recompilation,
the compiler might crash (it depends on the order of input files), since in
read_cgraph_and_symbols it expects that *all* input files contain IR section of
given type.
This patch splits input objects from argv into lto_argv and offload_argv, so
that all files in arrays contain corresponding IR.
Similarly, in lto-plugin, it was bad idea to add objects, which contain offload
IR without LTO, to claimed_files, since this may corrupt a resolution file.

Tested on various combinations of files with/without -flto and with/without
offload, using trunk ld and gold, also tested on ld without plugin support.
Bootstrap and make check passed on x86_64-linux and i686-linux.  Ok for trunk?

But I am unable to run lto-bootstrap on trunk, it says:
/x/build-x86_64-linux/./prev-gcc/gcc-ar -B/x/build-x86_64-linux/./prev-gcc/ cru libdecnumber.a decNumber.o decContext.o decimal32.o decimal64.o decimal128.o bid2dpd_dpd2bid.o host-ieee32.o host-ieee64.o host-ieee128.o
/usr/bin/ar terminated with signal 11 [Segmentation fault], core dumped
make[4]: *** [libdecnumber.a] Error 1


gcc/
	* lto-wrapper.c (compile_offload_image): Start processing in_argv
	from 0 instead of 1.
	(run_gcc): Put offload objects into offload_argv, put LTO objects and
	possible preceding arguments into lto_argv.
	Pass offload_argv to compile_images_for_offload_targets instead of argv.
	Use lto_argv for LTO recompilation instead of argv.
lto-plugin/
	* lto-plugin.c (offload_files, num_offload_files): New static variables.
	(free_1): Use arguments instead of global variables.
	(free_2): Free offload_files.
	(all_symbols_read_handler): Add names from offload_files to lto-wrapper
	arguments.
	(claim_file_handler): Do not add file to claimed_files if it contains
	offload sections without LTO sections.  Add it to offload_files instead.


diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 0f69457..f75c0dc 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -669,7 +669,7 @@ compile_offload_image (const char *target, const char *compiler_path,
       obstack_ptr_grow (&argv_obstack, filename);
 
       /* Append names of input object files.  */
-      for (unsigned i = 1; i < in_argc; i++)
+      for (unsigned i = 0; i < in_argc; i++)
 	obstack_ptr_grow (&argv_obstack, in_argv[i]);
 
       /* Append options from offload_lto sections.  */
@@ -883,6 +883,8 @@ run_gcc (unsigned argc, char *argv[])
   int new_head_argc;
   bool have_lto = false;
   bool have_offload = false;
+  unsigned lto_argc = 0, offload_argc = 0;
+  char **lto_argv, **offload_argv;
 
   /* Get the driver and options.  */
   collect_gcc = getenv ("COLLECT_GCC");
@@ -896,6 +898,11 @@ run_gcc (unsigned argc, char *argv[])
 					&decoded_options,
 					&decoded_options_count);
 
+  /* Allocate arrays for input object files with LTO or offload IL,
+     and for possible preceding arguments.  */
+  lto_argv = XNEWVEC (char *, argc);
+  offload_argv = XNEWVEC (char *, argc);
+
   /* Look at saved options in the IL files.  */
   for (i = 1; i < argc; ++i)
     {
@@ -918,17 +925,27 @@ run_gcc (unsigned argc, char *argv[])
 	}
       fd = open (argv[i], O_RDONLY);
       if (fd == -1)
-	continue;
+	{
+	  lto_argv[lto_argc++] = argv[i];
+	  continue;
+	}
+
+      if (find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
+				  &fdecoded_options, &fdecoded_options_count,
+				  collect_gcc))
+	{
+	  have_lto = true;
+	  lto_argv[lto_argc++] = argv[i];
+	}
+
+      if (find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
+				  &offload_fdecoded_options,
+				  &offload_fdecoded_options_count, collect_gcc))
+	{
+	  have_offload = true;
+	  offload_argv[offload_argc++] = argv[i];
+	}
 
-      have_lto
-	|= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
-				   &fdecoded_options, &fdecoded_options_count,
-				   collect_gcc);
-      have_offload
-	|= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
-				   &offload_fdecoded_options,
-				   &offload_fdecoded_options_count,
-				   collect_gcc);
       close (fd);
     }
 
@@ -1027,7 +1044,8 @@ run_gcc (unsigned argc, char *argv[])
 
   if (have_offload)
     {
-      compile_images_for_offload_targets (argc, argv, offload_fdecoded_options,
+      compile_images_for_offload_targets (offload_argc, offload_argv,
+					  offload_fdecoded_options,
 					  offload_fdecoded_options_count,
 					  decoded_options,
 					  decoded_options_count);
@@ -1119,8 +1137,8 @@ run_gcc (unsigned argc, char *argv[])
     }
 
   /* Append the input objects and possible preceding arguments.  */
-  for (i = 1; i < argc; ++i)
-    obstack_ptr_grow (&argv_obstack, argv[i]);
+  for (i = 0; i < lto_argc; ++i)
+    obstack_ptr_grow (&argv_obstack, lto_argv[i]);
   obstack_ptr_grow (&argv_obstack, NULL);
 
   new_argv = XOBFINISH (&argv_obstack, const char **);
@@ -1295,6 +1313,8 @@ cont:
   if (offloadend)
     printf ("%s\n", offloadend);
 
+  XDELETE (lto_argv);
+  XDELETE (offload_argv);
   obstack_free (&argv_obstack, NULL);
 }
 
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index fb6555d..8d957402 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -152,6 +152,9 @@ static ld_plugin_add_symbols add_symbols;
 static struct plugin_file_info *claimed_files = NULL;
 static unsigned int num_claimed_files = 0;
 
+static struct plugin_file_info *offload_files = NULL;
+static unsigned int num_offload_files = 0;
+
 static char **output_files = NULL;
 static unsigned int num_output_files = 0;
 
@@ -313,12 +316,12 @@ translate (char *data, char *end, struct plugin_symtab *out)
    resolution. */
 
 static void
-free_1 (void)
+free_1 (struct plugin_file_info *files, unsigned num_files)
 {
   unsigned int i;
-  for (i = 0; i < num_claimed_files; i++)
+  for (i = 0; i < num_files; i++)
     {
-      struct plugin_file_info *info = &claimed_files[i];
+      struct plugin_file_info *info = &files[i];
       struct plugin_symtab *symtab = &info->symtab;
       unsigned int j;
       for (j = 0; j < symtab->nsyms; j++)
@@ -346,6 +349,14 @@ free_2 (void)
       free (info->name);
     }
 
+  for (i = 0; i < num_offload_files; i++)
+    {
+      struct plugin_file_info *info = &offload_files[i];
+      struct plugin_symtab *symtab = &info->symtab;
+      free (symtab->aux);
+      free (info->name);
+    }
+
   for (i = 0; i < num_output_files; i++)
     free (output_files[i]);
   free (output_files);
@@ -354,6 +365,10 @@ free_2 (void)
   claimed_files = NULL;
   num_claimed_files = 0;
 
+  free (offload_files);
+  offload_files = NULL;
+  num_offload_files = 0;
+
   free (arguments_file_name);
   arguments_file_name = NULL;
 }
@@ -608,10 +623,11 @@ static enum ld_plugin_status
 all_symbols_read_handler (void)
 {
   unsigned i;
-  unsigned num_lto_args = num_claimed_files + lto_wrapper_num_args + 1;
+  unsigned num_lto_args
+    = num_claimed_files + num_offload_files + lto_wrapper_num_args + 1;
   char **lto_argv;
   const char **lto_arg_ptr;
-  if (num_claimed_files == 0)
+  if (num_claimed_files + num_offload_files == 0)
     return LDPS_OK;
 
   if (nop)
@@ -626,7 +642,8 @@ all_symbols_read_handler (void)
 
   write_resolution ();
 
-  free_1 ();
+  free_1 (claimed_files, num_claimed_files);
+  free_1 (offload_files, num_offload_files);
 
   for (i = 0; i < lto_wrapper_num_args; i++)
     *lto_arg_ptr++ = lto_wrapper_argv[i];
@@ -638,6 +655,13 @@ all_symbols_read_handler (void)
       *lto_arg_ptr++ = info->name;
     }
 
+  for (i = 0; i < num_offload_files; i++)
+    {
+      struct plugin_file_info *info = &offload_files[i];
+
+      *lto_arg_ptr++ = info->name;
+    }
+
   *lto_arg_ptr++ = NULL;
   exec_lto_wrapper (lto_argv);
 
@@ -949,16 +973,29 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
   if (obj.found > 1)
     resolve_conflicts (&lto_file.symtab, &lto_file.conflicts);
 
-  status = add_symbols (file->handle, lto_file.symtab.nsyms,
-			lto_file.symtab.syms);
-  check (status == LDPS_OK, LDPL_FATAL, "could not add symbols");
+  if (obj.found > 0)
+    {
+      status = add_symbols (file->handle, lto_file.symtab.nsyms,
+			    lto_file.symtab.syms);
+      check (status == LDPS_OK, LDPL_FATAL, "could not add symbols");
+
+      num_claimed_files++;
+      claimed_files =
+	xrealloc (claimed_files,
+		  num_claimed_files * sizeof (struct plugin_file_info));
+      claimed_files[num_claimed_files - 1] = lto_file;
+    }
+
+  if (obj.found == 0 && obj.offload == 1)
+    {
+      num_offload_files++;
+      offload_files =
+	xrealloc (offload_files,
+		  num_offload_files * sizeof (struct plugin_file_info));
+      offload_files[num_offload_files - 1] = lto_file;
+    }
 
   *claimed = 1;
-  num_claimed_files++;
-  claimed_files =
-    xrealloc (claimed_files,
-	      num_claimed_files * sizeof (struct plugin_file_info));
-  claimed_files[num_claimed_files - 1] = lto_file;
 
   goto cleanup;
 

  -- Ilya

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

[Richard Biener]

On Mon, 8 Dec 2014, Ilya Verbin wrote:

> Hi,
> 
> On 28 Nov 09:36, Richard Biener wrote:
> > On Fri, 28 Nov 2014, Ilya Verbin wrote:
> > > I found a bug here, have_{lto,offload} must be set if at least one file contains
> > > lto/offload sections, but currently they are overwritten by the last file.
> > > Fix is bootstrapped and regtested on x86_64-linux.  OK for trunk?
> > Ok.
> 
> Unfortunately, this fix was not general enough.
> There might be cases when mixed object files get into lto-wrapper, ie some of
> them contain only LTO sections, some contain only offload sections, and some
> contain both.  But when lto-wrapper will pass all these files to recompilation,
> the compiler might crash (it depends on the order of input files), since in
> read_cgraph_and_symbols it expects that *all* input files contain IR section of
> given type.
> This patch splits input objects from argv into lto_argv and offload_argv, so
> that all files in arrays contain corresponding IR.
> Similarly, in lto-plugin, it was bad idea to add objects, which contain offload
> IR without LTO, to claimed_files, since this may corrupt a resolution file.
> 
> Tested on various combinations of files with/without -flto and with/without
> offload, using trunk ld and gold, also tested on ld without plugin support.
> Bootstrap and make check passed on x86_64-linux and i686-linux.  Ok for trunk?

Did you check that bootstrap-lto still works?  Ok if so.

Thanks,
Richard.

> But I am unable to run lto-bootstrap on trunk, it says:
> /x/build-x86_64-linux/./prev-gcc/gcc-ar -B/x/build-x86_64-linux/./prev-gcc/ cru libdecnumber.a decNumber.o decContext.o decimal32.o decimal64.o decimal128.o bid2dpd_dpd2bid.o host-ieee32.o host-ieee64.o host-ieee128.o
> /usr/bin/ar terminated with signal 11 [Segmentation fault], core dumped
> make[4]: *** [libdecnumber.a] Error 1
> 
> 
> gcc/
> 	* lto-wrapper.c (compile_offload_image): Start processing in_argv
> 	from 0 instead of 1.
> 	(run_gcc): Put offload objects into offload_argv, put LTO objects and
> 	possible preceding arguments into lto_argv.
> 	Pass offload_argv to compile_images_for_offload_targets instead of argv.
> 	Use lto_argv for LTO recompilation instead of argv.
> lto-plugin/
> 	* lto-plugin.c (offload_files, num_offload_files): New static variables.
> 	(free_1): Use arguments instead of global variables.
> 	(free_2): Free offload_files.
> 	(all_symbols_read_handler): Add names from offload_files to lto-wrapper
> 	arguments.
> 	(claim_file_handler): Do not add file to claimed_files if it contains
> 	offload sections without LTO sections.  Add it to offload_files instead.
> 
> 
> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index 0f69457..f75c0dc 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -669,7 +669,7 @@ compile_offload_image (const char *target, const char *compiler_path,
>        obstack_ptr_grow (&argv_obstack, filename);
>  
>        /* Append names of input object files.  */
> -      for (unsigned i = 1; i < in_argc; i++)
> +      for (unsigned i = 0; i < in_argc; i++)
>  	obstack_ptr_grow (&argv_obstack, in_argv[i]);
>  
>        /* Append options from offload_lto sections.  */
> @@ -883,6 +883,8 @@ run_gcc (unsigned argc, char *argv[])
>    int new_head_argc;
>    bool have_lto = false;
>    bool have_offload = false;
> +  unsigned lto_argc = 0, offload_argc = 0;
> +  char **lto_argv, **offload_argv;
>  
>    /* Get the driver and options.  */
>    collect_gcc = getenv ("COLLECT_GCC");
> @@ -896,6 +898,11 @@ run_gcc (unsigned argc, char *argv[])
>  					&decoded_options,
>  					&decoded_options_count);
>  
> +  /* Allocate arrays for input object files with LTO or offload IL,
> +     and for possible preceding arguments.  */
> +  lto_argv = XNEWVEC (char *, argc);
> +  offload_argv = XNEWVEC (char *, argc);
> +
>    /* Look at saved options in the IL files.  */
>    for (i = 1; i < argc; ++i)
>      {
> @@ -918,17 +925,27 @@ run_gcc (unsigned argc, char *argv[])
>  	}
>        fd = open (argv[i], O_RDONLY);
>        if (fd == -1)
> -	continue;
> +	{
> +	  lto_argv[lto_argc++] = argv[i];
> +	  continue;
> +	}
> +
> +      if (find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
> +				  &fdecoded_options, &fdecoded_options_count,
> +				  collect_gcc))
> +	{
> +	  have_lto = true;
> +	  lto_argv[lto_argc++] = argv[i];
> +	}
> +
> +      if (find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
> +				  &offload_fdecoded_options,
> +				  &offload_fdecoded_options_count, collect_gcc))
> +	{
> +	  have_offload = true;
> +	  offload_argv[offload_argc++] = argv[i];
> +	}
>  
> -      have_lto
> -	|= find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
> -				   &fdecoded_options, &fdecoded_options_count,
> -				   collect_gcc);
> -      have_offload
> -	|= find_and_merge_options (fd, file_offset, OFFLOAD_SECTION_NAME_PREFIX,
> -				   &offload_fdecoded_options,
> -				   &offload_fdecoded_options_count,
> -				   collect_gcc);
>        close (fd);
>      }
>  
> @@ -1027,7 +1044,8 @@ run_gcc (unsigned argc, char *argv[])
>  
>    if (have_offload)
>      {
> -      compile_images_for_offload_targets (argc, argv, offload_fdecoded_options,
> +      compile_images_for_offload_targets (offload_argc, offload_argv,
> +					  offload_fdecoded_options,
>  					  offload_fdecoded_options_count,
>  					  decoded_options,
>  					  decoded_options_count);
> @@ -1119,8 +1137,8 @@ run_gcc (unsigned argc, char *argv[])
>      }
>  
>    /* Append the input objects and possible preceding arguments.  */
> -  for (i = 1; i < argc; ++i)
> -    obstack_ptr_grow (&argv_obstack, argv[i]);
> +  for (i = 0; i < lto_argc; ++i)
> +    obstack_ptr_grow (&argv_obstack, lto_argv[i]);
>    obstack_ptr_grow (&argv_obstack, NULL);
>  
>    new_argv = XOBFINISH (&argv_obstack, const char **);
> @@ -1295,6 +1313,8 @@ cont:
>    if (offloadend)
>      printf ("%s\n", offloadend);
>  
> +  XDELETE (lto_argv);
> +  XDELETE (offload_argv);
>    obstack_free (&argv_obstack, NULL);
>  }
>  
> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> index fb6555d..8d957402 100644
> --- a/lto-plugin/lto-plugin.c
> +++ b/lto-plugin/lto-plugin.c
> @@ -152,6 +152,9 @@ static ld_plugin_add_symbols add_symbols;
>  static struct plugin_file_info *claimed_files = NULL;
>  static unsigned int num_claimed_files = 0;
>  
> +static struct plugin_file_info *offload_files = NULL;
> +static unsigned int num_offload_files = 0;
> +
>  static char **output_files = NULL;
>  static unsigned int num_output_files = 0;
>  
> @@ -313,12 +316,12 @@ translate (char *data, char *end, struct plugin_symtab *out)
>     resolution. */
>  
>  static void
> -free_1 (void)
> +free_1 (struct plugin_file_info *files, unsigned num_files)
>  {
>    unsigned int i;
> -  for (i = 0; i < num_claimed_files; i++)
> +  for (i = 0; i < num_files; i++)
>      {
> -      struct plugin_file_info *info = &claimed_files[i];
> +      struct plugin_file_info *info = &files[i];
>        struct plugin_symtab *symtab = &info->symtab;
>        unsigned int j;
>        for (j = 0; j < symtab->nsyms; j++)
> @@ -346,6 +349,14 @@ free_2 (void)
>        free (info->name);
>      }
>  
> +  for (i = 0; i < num_offload_files; i++)
> +    {
> +      struct plugin_file_info *info = &offload_files[i];
> +      struct plugin_symtab *symtab = &info->symtab;
> +      free (symtab->aux);
> +      free (info->name);
> +    }
> +
>    for (i = 0; i < num_output_files; i++)
>      free (output_files[i]);
>    free (output_files);
> @@ -354,6 +365,10 @@ free_2 (void)
>    claimed_files = NULL;
>    num_claimed_files = 0;
>  
> +  free (offload_files);
> +  offload_files = NULL;
> +  num_offload_files = 0;
> +
>    free (arguments_file_name);
>    arguments_file_name = NULL;
>  }
> @@ -608,10 +623,11 @@ static enum ld_plugin_status
>  all_symbols_read_handler (void)
>  {
>    unsigned i;
> -  unsigned num_lto_args = num_claimed_files + lto_wrapper_num_args + 1;
> +  unsigned num_lto_args
> +    = num_claimed_files + num_offload_files + lto_wrapper_num_args + 1;
>    char **lto_argv;
>    const char **lto_arg_ptr;
> -  if (num_claimed_files == 0)
> +  if (num_claimed_files + num_offload_files == 0)
>      return LDPS_OK;
>  
>    if (nop)
> @@ -626,7 +642,8 @@ all_symbols_read_handler (void)
>  
>    write_resolution ();
>  
> -  free_1 ();
> +  free_1 (claimed_files, num_claimed_files);
> +  free_1 (offload_files, num_offload_files);
>  
>    for (i = 0; i < lto_wrapper_num_args; i++)
>      *lto_arg_ptr++ = lto_wrapper_argv[i];
> @@ -638,6 +655,13 @@ all_symbols_read_handler (void)
>        *lto_arg_ptr++ = info->name;
>      }
>  
> +  for (i = 0; i < num_offload_files; i++)
> +    {
> +      struct plugin_file_info *info = &offload_files[i];
> +
> +      *lto_arg_ptr++ = info->name;
> +    }
> +
>    *lto_arg_ptr++ = NULL;
>    exec_lto_wrapper (lto_argv);
>  
> @@ -949,16 +973,29 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
>    if (obj.found > 1)
>      resolve_conflicts (&lto_file.symtab, &lto_file.conflicts);
>  
> -  status = add_symbols (file->handle, lto_file.symtab.nsyms,
> -			lto_file.symtab.syms);
> -  check (status == LDPS_OK, LDPL_FATAL, "could not add symbols");
> +  if (obj.found > 0)
> +    {
> +      status = add_symbols (file->handle, lto_file.symtab.nsyms,
> +			    lto_file.symtab.syms);
> +      check (status == LDPS_OK, LDPL_FATAL, "could not add symbols");
> +
> +      num_claimed_files++;
> +      claimed_files =
> +	xrealloc (claimed_files,
> +		  num_claimed_files * sizeof (struct plugin_file_info));
> +      claimed_files[num_claimed_files - 1] = lto_file;
> +    }
> +
> +  if (obj.found == 0 && obj.offload == 1)
> +    {
> +      num_offload_files++;
> +      offload_files =
> +	xrealloc (offload_files,
> +		  num_offload_files * sizeof (struct plugin_file_info));
> +      offload_files[num_offload_files - 1] = lto_file;
> +    }
>  
>    *claimed = 1;
> -  num_claimed_files++;
> -  claimed_files =
> -    xrealloc (claimed_files,
> -	      num_claimed_files * sizeof (struct plugin_file_info));
> -  claimed_files[num_claimed_files - 1] = lto_file;
>  
>    goto cleanup;
>  
> 
>   -- Ilya
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany

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

[Ilya Verbin]

On 09 Dec 14:59, Richard Biener wrote:
> On Mon, 8 Dec 2014, Ilya Verbin wrote:
> > Unfortunately, this fix was not general enough.
> > There might be cases when mixed object files get into lto-wrapper, ie some of
> > them contain only LTO sections, some contain only offload sections, and some
> > contain both.  But when lto-wrapper will pass all these files to recompilation,
> > the compiler might crash (it depends on the order of input files), since in
> > read_cgraph_and_symbols it expects that *all* input files contain IR section of
> > given type.
> > This patch splits input objects from argv into lto_argv and offload_argv, so
> > that all files in arrays contain corresponding IR.
> > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload
> > IR without LTO, to claimed_files, since this may corrupt a resolution file.
> > 
> > Tested on various combinations of files with/without -flto and with/without
> > offload, using trunk ld and gold, also tested on ld without plugin support.
> > Bootstrap and make check passed on x86_64-linux and i686-linux.  Ok for trunk?
> 
> Did you check that bootstrap-lto still works?  Ok if so.

Yes, bootstrap-lto passed.
Committed revision 218543.

Thanks,
  -- Ilya

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

[Thomas Schwinge]

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

Hi!

On Mon, 13 Oct 2014 14:33:11 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> On 13 Oct 12:19, Jakub Jelinek wrote:
> > On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> > > 2. -foffload-abi=[lp64|ilp32]
> > >    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> > > used in streamed GIMPLE.  This option is desirable, because host and offload
> > > compilers must have the same ABI.  The option is generated by the host compiler
> > > automatically, it should not be specified by user.
> > 
> > But I'd like to understand why is this one needed.
> > Why should the compilers care?  Aggregates layout and alignment of
> > integral/floating types must match between host and offload compilers, sure,
> > but isn't that something streamed already in the LTO bytecode?
> > Or is LTO streamer not streaming some types like long_type_node?
> > I'd expect if host and offload compiler disagree on long type size that
> > you'd just use a different integral type with the same size as long on the
> > host.
> > Different sized pointers are of course a bigger problem, but can't you just
> > error out on that during reading of the LTO, or even handle it (just use
> > some integral type for when is the pointer stored in memory, and just
> > convert to pointer after reads from memory, and convert back before storing
> > to memory).  Erroring out during LTO streaming in sounds just fine to me
> > though.
> 
> Actually this option was developed by Bernd, so I think PTX team is going to use
> it somehow.  In MIC's case we're planning just to check in mkoffload that host
> and target compiler's ABI are the same.  Without this check we will crash in LTO
> streamer with ICE, so I'd like to issue an error message, rather than crashing.

In gcc/config/i386/intelmic-mkoffload.c, this option is now parsed to
initialize the target_ilp32 variable, which will then be used
(target_ilp32 ? "-m32" : "-m64") when invoking different tools.

In nvptx, we've been using the following approach:

--- gcc/config/nvptx/nvptx.h
+++ gcc/config/nvptx/nvptx.h
@@ -54,24 +54,28 @@
 
 /* Type Layout.  */
 
+#define TARGET_64BIT \
+  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
+   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
+
 #define DEFAULT_SIGNED_CHAR 1
 
 #define SHORT_TYPE_SIZE 16
 #define INT_TYPE_SIZE 32
-#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
+#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
 #define LONG_LONG_TYPE_SIZE 64
 #define FLOAT_TYPE_SIZE 32
 #define DOUBLE_TYPE_SIZE 64
 #define LONG_DOUBLE_TYPE_SIZE 64
 
 #undef SIZE_TYPE
-#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
+#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
 #undef PTRDIFF_TYPE
-#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
+#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
 
-#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
+#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
 
-#define Pmode (TARGET_ABI64 ? DImode : SImode)
+#define Pmode (TARGET_64BIT ? DImode : SImode)
 
 /* Registers.  Since ptx is a virtual target, we just define a few
    hard registers for special purposes and leave pseudos unallocated.  */

Should we settle on one of the two, that is, either pass -m[...] from
mkoffload, or handle flag_offload_abi in the respective backend?  I think
I prefer the intelmic-mkoffload.c approach; this seems cleaner to me:
mkoffload "configures" the offloading compiler.  (Also, the flag 32-bit
vs. 64-bit flag may in fact be needed for tools other than the offloading
compiler).  Bernd, is there any specific reason for the approach you had
chosen?


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

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

[Thomas Schwinge]

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

Hi!

On Wed, 18 Feb 2015 18:04:21 +0100, I wrote:
> On Mon, 13 Oct 2014 14:33:11 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > On 13 Oct 12:19, Jakub Jelinek wrote:
> > > On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> > > > 2. -foffload-abi=[lp64|ilp32]
> > > >    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> > > > used in streamed GIMPLE.  This option is desirable, because host and offload
> > > > compilers must have the same ABI.  The option is generated by the host compiler
> > > > automatically, it should not be specified by user.
> > > 
> > > But I'd like to understand why is this one needed.
> > > Why should the compilers care?  Aggregates layout and alignment of
> > > integral/floating types must match between host and offload compilers, sure,
> > > but isn't that something streamed already in the LTO bytecode?
> > > Or is LTO streamer not streaming some types like long_type_node?
> > > I'd expect if host and offload compiler disagree on long type size that
> > > you'd just use a different integral type with the same size as long on the
> > > host.
> > > Different sized pointers are of course a bigger problem, but can't you just
> > > error out on that during reading of the LTO, or even handle it (just use
> > > some integral type for when is the pointer stored in memory, and just
> > > convert to pointer after reads from memory, and convert back before storing
> > > to memory).  Erroring out during LTO streaming in sounds just fine to me
> > > though.
> > 
> > Actually this option was developed by Bernd, so I think PTX team is going to use
> > it somehow.  In MIC's case we're planning just to check in mkoffload that host
> > and target compiler's ABI are the same.  Without this check we will crash in LTO
> > streamer with ICE, so I'd like to issue an error message, rather than crashing.
> 
> In gcc/config/i386/intelmic-mkoffload.c, this option is now parsed to
> initialize the target_ilp32 variable, which will then be used
> (target_ilp32 ? "-m32" : "-m64") when invoking different tools.
> 
> In nvptx, we've been using the following approach:
> 
> --- gcc/config/nvptx/nvptx.h
> +++ gcc/config/nvptx/nvptx.h
> @@ -54,24 +54,28 @@
>  
>  /* Type Layout.  */
>  
> +#define TARGET_64BIT \
> +  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
> +   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
> +
>  #define DEFAULT_SIGNED_CHAR 1
>  
>  #define SHORT_TYPE_SIZE 16
>  #define INT_TYPE_SIZE 32
> -#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
> +#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
>  #define LONG_LONG_TYPE_SIZE 64
>  #define FLOAT_TYPE_SIZE 32
>  #define DOUBLE_TYPE_SIZE 64
>  #define LONG_DOUBLE_TYPE_SIZE 64
>  
>  #undef SIZE_TYPE
> -#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
> +#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
>  #undef PTRDIFF_TYPE
> -#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
> +#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
>  
> -#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
> +#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
>  
> -#define Pmode (TARGET_ABI64 ? DImode : SImode)
> +#define Pmode (TARGET_64BIT ? DImode : SImode)
>  
>  /* Registers.  Since ptx is a virtual target, we just define a few
>     hard registers for special purposes and leave pseudos unallocated.  */
> 
> Should we settle on one of the two, that is, either pass -m[...] from
> mkoffload, or handle flag_offload_abi in the respective backend?  I think
> I prefer the intelmic-mkoffload.c approach; this seems cleaner to me:
> mkoffload "configures" the offloading compiler.  (Also, the flag 32-bit
> vs. 64-bit flag may in fact be needed for tools other than the offloading
> compiler).

Well, "for instance", when in -m32 mode, we also need to use it in
gcc/config/nvptx/mkoffload.c:compile_native -- otherwise, later on, the
linker will not be happy with the request to link a 64-bit object file
with 32-bit object files...

> Bernd, is there any specific reason for the approach you had
> chosen?

OK to do the following instead?  (Coding style/code copied from
gcc/config/i386/intelmic-mkoffload.c for consistency.)

--- gcc/config/nvptx/mkoffload.c
+++ gcc/config/nvptx/mkoffload.c
@@ -126,6 +126,9 @@ static id_map *var_ids, **vars_tail = &var_ids;
 static const char *ptx_name;
 static const char *ptx_cfile_name;
 
+/* Shows if we should compile binaries for i386 instead of x86-64.  */
+bool target_ilp32 = false;
+
 /* Delete tempfiles.  */
 
 /* Unlink a temporary file unless requested otherwise.  */
@@ -883,6 +886,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, compiler);
+  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
   obstack_ptr_grow (&argv_obstack, infile);
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-o");
@@ -960,10 +964,23 @@ main (int argc, char **argv)
      passed with @file.  Expand them into argv before processing.  */
   expandargv (&argc, &argv);
 
+  /* Find out whether we should compile binaries for i386 or x86-64.  */
+  for (int i = argc - 1; i > 0; i--)
+    if (strncmp (argv[i], "-foffload-abi=", sizeof ("-foffload-abi=") - 1) == 0)
+      {
+	if (strstr (argv[i], "ilp32"))
+	  target_ilp32 = true;
+	else if (!strstr (argv[i], "lp64"))
+	  fatal_error (input_location,
+		       "unrecognizable argument of option -foffload-abi");
+	break;
+      }
+
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, driver);
   obstack_ptr_grow (&argv_obstack, "-xlto");
+  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
   obstack_ptr_grow (&argv_obstack, "-S");
 
   for (int ix = 1; ix != argc; ix++)
diff --git gcc/config/nvptx/nvptx.h gcc/config/nvptx/nvptx.h
index 15460b7..e74d16f 100644
--- gcc/config/nvptx/nvptx.h
+++ gcc/config/nvptx/nvptx.h
@@ -58,28 +58,24 @@
 
 /* Type Layout.  */
 
-#define TARGET_64BIT \
-  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
-   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
-
 #define DEFAULT_SIGNED_CHAR 1
 
 #define SHORT_TYPE_SIZE 16
 #define INT_TYPE_SIZE 32
-#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
+#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
 #define LONG_LONG_TYPE_SIZE 64
 #define FLOAT_TYPE_SIZE 32
 #define DOUBLE_TYPE_SIZE 64
 #define LONG_DOUBLE_TYPE_SIZE 64
 
 #undef SIZE_TYPE
-#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
+#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
 #undef PTRDIFF_TYPE
-#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
+#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
 
-#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
+#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
 
-#define Pmode (TARGET_64BIT ? DImode : SImode)
+#define Pmode (TARGET_ABI64 ? DImode : SImode)
 
 /* Registers.  Since ptx is a virtual target, we just define a few
    hard registers for special purposes and leave pseudos unallocated.  */


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

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

[Thomas Schwinge]

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

Hi!

Ping.

On Thu, 19 Feb 2015 09:39:52 +0100, I wrote:
> On Wed, 18 Feb 2015 18:04:21 +0100, I wrote:
> > On Mon, 13 Oct 2014 14:33:11 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > > On 13 Oct 12:19, Jakub Jelinek wrote:
> > > > On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> > > > > 2. -foffload-abi=[lp64|ilp32]
> > > > >    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> > > > > used in streamed GIMPLE.  This option is desirable, because host and offload
> > > > > compilers must have the same ABI.  The option is generated by the host compiler
> > > > > automatically, it should not be specified by user.
> > > > 
> > > > But I'd like to understand why is this one needed.
> > > > Why should the compilers care?  Aggregates layout and alignment of
> > > > integral/floating types must match between host and offload compilers, sure,
> > > > but isn't that something streamed already in the LTO bytecode?
> > > > Or is LTO streamer not streaming some types like long_type_node?
> > > > I'd expect if host and offload compiler disagree on long type size that
> > > > you'd just use a different integral type with the same size as long on the
> > > > host.
> > > > Different sized pointers are of course a bigger problem, but can't you just
> > > > error out on that during reading of the LTO, or even handle it (just use
> > > > some integral type for when is the pointer stored in memory, and just
> > > > convert to pointer after reads from memory, and convert back before storing
> > > > to memory).  Erroring out during LTO streaming in sounds just fine to me
> > > > though.
> > > 
> > > Actually this option was developed by Bernd, so I think PTX team is going to use
> > > it somehow.  In MIC's case we're planning just to check in mkoffload that host
> > > and target compiler's ABI are the same.  Without this check we will crash in LTO
> > > streamer with ICE, so I'd like to issue an error message, rather than crashing.
> > 
> > In gcc/config/i386/intelmic-mkoffload.c, this option is now parsed to
> > initialize the target_ilp32 variable, which will then be used
> > (target_ilp32 ? "-m32" : "-m64") when invoking different tools.
> > 
> > In nvptx, we've been using the following approach:
> > 
> > --- gcc/config/nvptx/nvptx.h
> > +++ gcc/config/nvptx/nvptx.h
> > @@ -54,24 +54,28 @@
> >  
> >  /* Type Layout.  */
> >  
> > +#define TARGET_64BIT \
> > +  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
> > +   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
> > +
> >  #define DEFAULT_SIGNED_CHAR 1
> >  
> >  #define SHORT_TYPE_SIZE 16
> >  #define INT_TYPE_SIZE 32
> > -#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
> > +#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
> >  #define LONG_LONG_TYPE_SIZE 64
> >  #define FLOAT_TYPE_SIZE 32
> >  #define DOUBLE_TYPE_SIZE 64
> >  #define LONG_DOUBLE_TYPE_SIZE 64
> >  
> >  #undef SIZE_TYPE
> > -#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
> > +#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
> >  #undef PTRDIFF_TYPE
> > -#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
> > +#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
> >  
> > -#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
> > +#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
> >  
> > -#define Pmode (TARGET_ABI64 ? DImode : SImode)
> > +#define Pmode (TARGET_64BIT ? DImode : SImode)
> >  
> >  /* Registers.  Since ptx is a virtual target, we just define a few
> >     hard registers for special purposes and leave pseudos unallocated.  */
> > 
> > Should we settle on one of the two, that is, either pass -m[...] from
> > mkoffload, or handle flag_offload_abi in the respective backend?  I think
> > I prefer the intelmic-mkoffload.c approach; this seems cleaner to me:
> > mkoffload "configures" the offloading compiler.  (Also, the flag 32-bit
> > vs. 64-bit flag may in fact be needed for tools other than the offloading
> > compiler).
> 
> Well, "for instance", when in -m32 mode, we also need to use it in
> gcc/config/nvptx/mkoffload.c:compile_native -- otherwise, later on, the
> linker will not be happy with the request to link a 64-bit object file
> with 32-bit object files...
> 
> > Bernd, is there any specific reason for the approach you had
> > chosen?
> 
> OK to do the following instead?  (Coding style/code copied from
> gcc/config/i386/intelmic-mkoffload.c for consistency.)
> 
> --- gcc/config/nvptx/mkoffload.c
> +++ gcc/config/nvptx/mkoffload.c
> @@ -126,6 +126,9 @@ static id_map *var_ids, **vars_tail = &var_ids;
>  static const char *ptx_name;
>  static const char *ptx_cfile_name;
>  
> +/* Shows if we should compile binaries for i386 instead of x86-64.  */
> +bool target_ilp32 = false;
> +
>  /* Delete tempfiles.  */
>  
>  /* Unlink a temporary file unless requested otherwise.  */
> @@ -883,6 +886,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
>    struct obstack argv_obstack;
>    obstack_init (&argv_obstack);
>    obstack_ptr_grow (&argv_obstack, compiler);
> +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
>    obstack_ptr_grow (&argv_obstack, infile);
>    obstack_ptr_grow (&argv_obstack, "-c");
>    obstack_ptr_grow (&argv_obstack, "-o");
> @@ -960,10 +964,23 @@ main (int argc, char **argv)
>       passed with @file.  Expand them into argv before processing.  */
>    expandargv (&argc, &argv);
>  
> +  /* Find out whether we should compile binaries for i386 or x86-64.  */
> +  for (int i = argc - 1; i > 0; i--)
> +    if (strncmp (argv[i], "-foffload-abi=", sizeof ("-foffload-abi=") - 1) == 0)
> +      {
> +	if (strstr (argv[i], "ilp32"))
> +	  target_ilp32 = true;
> +	else if (!strstr (argv[i], "lp64"))
> +	  fatal_error (input_location,
> +		       "unrecognizable argument of option -foffload-abi");
> +	break;
> +      }
> +
>    struct obstack argv_obstack;
>    obstack_init (&argv_obstack);
>    obstack_ptr_grow (&argv_obstack, driver);
>    obstack_ptr_grow (&argv_obstack, "-xlto");
> +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
>    obstack_ptr_grow (&argv_obstack, "-S");
>  
>    for (int ix = 1; ix != argc; ix++)
> diff --git gcc/config/nvptx/nvptx.h gcc/config/nvptx/nvptx.h
> index 15460b7..e74d16f 100644
> --- gcc/config/nvptx/nvptx.h
> +++ gcc/config/nvptx/nvptx.h
> @@ -58,28 +58,24 @@
>  
>  /* Type Layout.  */
>  
> -#define TARGET_64BIT \
> -  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
> -   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
> -
>  #define DEFAULT_SIGNED_CHAR 1
>  
>  #define SHORT_TYPE_SIZE 16
>  #define INT_TYPE_SIZE 32
> -#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
> +#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
>  #define LONG_LONG_TYPE_SIZE 64
>  #define FLOAT_TYPE_SIZE 32
>  #define DOUBLE_TYPE_SIZE 64
>  #define LONG_DOUBLE_TYPE_SIZE 64
>  
>  #undef SIZE_TYPE
> -#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
> +#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
>  #undef PTRDIFF_TYPE
> -#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
> +#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
>  
> -#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
> +#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
>  
> -#define Pmode (TARGET_64BIT ? DImode : SImode)
> +#define Pmode (TARGET_ABI64 ? DImode : SImode)
>  
>  /* Registers.  Since ptx is a virtual target, we just define a few
>     hard registers for special purposes and leave pseudos unallocated.  */


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

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

[Thomas Schwinge]

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

Hi!

Ping.

On Tue, 10 Mar 2015 13:37:47 +0100, I wrote:
> Ping.
> 
> On Thu, 19 Feb 2015 09:39:52 +0100, I wrote:
> > On Wed, 18 Feb 2015 18:04:21 +0100, I wrote:
> > > On Mon, 13 Oct 2014 14:33:11 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > > > On 13 Oct 12:19, Jakub Jelinek wrote:
> > > > > On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> > > > > > 2. -foffload-abi=[lp64|ilp32]
> > > > > >    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> > > > > > used in streamed GIMPLE.  This option is desirable, because host and offload
> > > > > > compilers must have the same ABI.  The option is generated by the host compiler
> > > > > > automatically, it should not be specified by user.
> > > > > 
> > > > > But I'd like to understand why is this one needed.
> > > > > Why should the compilers care?  Aggregates layout and alignment of
> > > > > integral/floating types must match between host and offload compilers, sure,
> > > > > but isn't that something streamed already in the LTO bytecode?
> > > > > Or is LTO streamer not streaming some types like long_type_node?
> > > > > I'd expect if host and offload compiler disagree on long type size that
> > > > > you'd just use a different integral type with the same size as long on the
> > > > > host.
> > > > > Different sized pointers are of course a bigger problem, but can't you just
> > > > > error out on that during reading of the LTO, or even handle it (just use
> > > > > some integral type for when is the pointer stored in memory, and just
> > > > > convert to pointer after reads from memory, and convert back before storing
> > > > > to memory).  Erroring out during LTO streaming in sounds just fine to me
> > > > > though.
> > > > 
> > > > Actually this option was developed by Bernd, so I think PTX team is going to use
> > > > it somehow.  In MIC's case we're planning just to check in mkoffload that host
> > > > and target compiler's ABI are the same.  Without this check we will crash in LTO
> > > > streamer with ICE, so I'd like to issue an error message, rather than crashing.
> > > 
> > > In gcc/config/i386/intelmic-mkoffload.c, this option is now parsed to
> > > initialize the target_ilp32 variable, which will then be used
> > > (target_ilp32 ? "-m32" : "-m64") when invoking different tools.
> > > 
> > > In nvptx, we've been using the following approach:
> > > 
> > > --- gcc/config/nvptx/nvptx.h
> > > +++ gcc/config/nvptx/nvptx.h
> > > @@ -54,24 +54,28 @@
> > >  
> > >  /* Type Layout.  */
> > >  
> > > +#define TARGET_64BIT \
> > > +  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
> > > +   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
> > > +
> > >  #define DEFAULT_SIGNED_CHAR 1
> > >  
> > >  #define SHORT_TYPE_SIZE 16
> > >  #define INT_TYPE_SIZE 32
> > > -#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
> > > +#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
> > >  #define LONG_LONG_TYPE_SIZE 64
> > >  #define FLOAT_TYPE_SIZE 32
> > >  #define DOUBLE_TYPE_SIZE 64
> > >  #define LONG_DOUBLE_TYPE_SIZE 64
> > >  
> > >  #undef SIZE_TYPE
> > > -#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
> > > +#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
> > >  #undef PTRDIFF_TYPE
> > > -#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
> > > +#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
> > >  
> > > -#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
> > > +#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
> > >  
> > > -#define Pmode (TARGET_ABI64 ? DImode : SImode)
> > > +#define Pmode (TARGET_64BIT ? DImode : SImode)
> > >  
> > >  /* Registers.  Since ptx is a virtual target, we just define a few
> > >     hard registers for special purposes and leave pseudos unallocated.  */
> > > 
> > > Should we settle on one of the two, that is, either pass -m[...] from
> > > mkoffload, or handle flag_offload_abi in the respective backend?  I think
> > > I prefer the intelmic-mkoffload.c approach; this seems cleaner to me:
> > > mkoffload "configures" the offloading compiler.  (Also, the flag 32-bit
> > > vs. 64-bit flag may in fact be needed for tools other than the offloading
> > > compiler).
> > 
> > Well, "for instance", when in -m32 mode, we also need to use it in
> > gcc/config/nvptx/mkoffload.c:compile_native -- otherwise, later on, the
> > linker will not be happy with the request to link a 64-bit object file
> > with 32-bit object files...
> > 
> > > Bernd, is there any specific reason for the approach you had
> > > chosen?
> > 
> > OK to do the following instead?  (Coding style/code copied from
> > gcc/config/i386/intelmic-mkoffload.c for consistency.)
> > 
> > --- gcc/config/nvptx/mkoffload.c
> > +++ gcc/config/nvptx/mkoffload.c
> > @@ -126,6 +126,9 @@ static id_map *var_ids, **vars_tail = &var_ids;
> >  static const char *ptx_name;
> >  static const char *ptx_cfile_name;
> >  
> > +/* Shows if we should compile binaries for i386 instead of x86-64.  */
> > +bool target_ilp32 = false;
> > +
> >  /* Delete tempfiles.  */
> >  
> >  /* Unlink a temporary file unless requested otherwise.  */
> > @@ -883,6 +886,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
> >    struct obstack argv_obstack;
> >    obstack_init (&argv_obstack);
> >    obstack_ptr_grow (&argv_obstack, compiler);
> > +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
> >    obstack_ptr_grow (&argv_obstack, infile);
> >    obstack_ptr_grow (&argv_obstack, "-c");
> >    obstack_ptr_grow (&argv_obstack, "-o");
> > @@ -960,10 +964,23 @@ main (int argc, char **argv)
> >       passed with @file.  Expand them into argv before processing.  */
> >    expandargv (&argc, &argv);
> >  
> > +  /* Find out whether we should compile binaries for i386 or x86-64.  */
> > +  for (int i = argc - 1; i > 0; i--)
> > +    if (strncmp (argv[i], "-foffload-abi=", sizeof ("-foffload-abi=") - 1) == 0)
> > +      {
> > +	if (strstr (argv[i], "ilp32"))
> > +	  target_ilp32 = true;
> > +	else if (!strstr (argv[i], "lp64"))
> > +	  fatal_error (input_location,
> > +		       "unrecognizable argument of option -foffload-abi");
> > +	break;
> > +      }
> > +
> >    struct obstack argv_obstack;
> >    obstack_init (&argv_obstack);
> >    obstack_ptr_grow (&argv_obstack, driver);
> >    obstack_ptr_grow (&argv_obstack, "-xlto");
> > +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
> >    obstack_ptr_grow (&argv_obstack, "-S");
> >  
> >    for (int ix = 1; ix != argc; ix++)
> > diff --git gcc/config/nvptx/nvptx.h gcc/config/nvptx/nvptx.h
> > index 15460b7..e74d16f 100644
> > --- gcc/config/nvptx/nvptx.h
> > +++ gcc/config/nvptx/nvptx.h
> > @@ -58,28 +58,24 @@
> >  
> >  /* Type Layout.  */
> >  
> > -#define TARGET_64BIT \
> > -  (flag_offload_abi == OFFLOAD_ABI_UNSET ? TARGET_ABI64 \
> > -   : flag_offload_abi == OFFLOAD_ABI_LP64 ? true : false)
> > -
> >  #define DEFAULT_SIGNED_CHAR 1
> >  
> >  #define SHORT_TYPE_SIZE 16
> >  #define INT_TYPE_SIZE 32
> > -#define LONG_TYPE_SIZE (TARGET_64BIT ? 64 : 32)
> > +#define LONG_TYPE_SIZE (TARGET_ABI64 ? 64 : 32)
> >  #define LONG_LONG_TYPE_SIZE 64
> >  #define FLOAT_TYPE_SIZE 32
> >  #define DOUBLE_TYPE_SIZE 64
> >  #define LONG_DOUBLE_TYPE_SIZE 64
> >  
> >  #undef SIZE_TYPE
> > -#define SIZE_TYPE (TARGET_64BIT ? "long unsigned int" : "unsigned int")
> > +#define SIZE_TYPE (TARGET_ABI64 ? "long unsigned int" : "unsigned int")
> >  #undef PTRDIFF_TYPE
> > -#define PTRDIFF_TYPE (TARGET_64BIT ? "long int" : "int")
> > +#define PTRDIFF_TYPE (TARGET_ABI64 ? "long int" : "int")
> >  
> > -#define POINTER_SIZE (TARGET_64BIT ? 64 : 32)
> > +#define POINTER_SIZE (TARGET_ABI64 ? 64 : 32)
> >  
> > -#define Pmode (TARGET_64BIT ? DImode : SImode)
> > +#define Pmode (TARGET_ABI64 ? DImode : SImode)
> >  
> >  /* Registers.  Since ptx is a virtual target, we just define a few
> >     hard registers for special purposes and leave pseudos unallocated.  */


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

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

[Bernd Schmidt]

On 04/27/2015 06:08 PM, Thomas Schwinge wrote:

>>> OK to do the following instead?  (Coding style/code copied from
>>> gcc/config/i386/intelmic-mkoffload.c for consistency.)

Err, was this a question for me? I'm fine with that too.


Bernd

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

[Thomas Schwinge]

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

Hi!

On Tue, 28 Apr 2015 15:24:08 +0200, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 04/27/2015 06:08 PM, Thomas Schwinge wrote:
> 
> >>> OK to do the following instead?  (Coding style/code copied from
> >>> gcc/config/i386/intelmic-mkoffload.c for consistency.)
> 
> Err, was this a question for me? I'm fine with that too.

Yes -- you're the nvptx maintainer after all.  ;-)

Committed in r222583:

commit df615909263269988fd9611f8d007902580829d9
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Apr 29 16:23:26 2015 +0000

    [PR libgomp/65099] nvptx mkoffload: pass "-m32" or "-m64" to the compiler
    
    ... depending on "-foffload-abi=[...]".
    
    Coding style/code copied from gcc/config/i386/intelmic-mkoffload.c for
    consistency.
    
    	gcc/
    	* config/nvptx/mkoffload.c (target_ilp32): New variable.
    	(main): Set it depending on "-foffload-abi=[...]".
    	(compile_native, main): Use it to pass "-m32" or "-m64" to the
    	compiler.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@222583 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog                |    8 ++++++++
 gcc/config/nvptx/mkoffload.c |   18 +++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index aaa06c3..d7455e4 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,11 @@
+2015-04-29  Thomas Schwinge  <thomas@codesourcery.com>
+
+	PR libgomp/65099
+	* config/nvptx/mkoffload.c (target_ilp32): New variable.
+	(main): Set it depending on "-foffload-abi=[...]".
+	(compile_native, main): Use it to pass "-m32" or "-m64" to the
+	compiler.
+
 2015-04-29  Alan Lawrence  <alan.lawrence@arm.com>
 
 	PR target/65770
diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
index dbc68bc..8687154 100644
--- gcc/config/nvptx/mkoffload.c
+++ gcc/config/nvptx/mkoffload.c
@@ -126,6 +126,9 @@ static id_map *var_ids, **vars_tail = &var_ids;
 static const char *ptx_name;
 static const char *ptx_cfile_name;
 
+/* Shows if we should compile binaries for i386 instead of x86-64.  */
+bool target_ilp32 = false;
+
 /* Delete tempfiles.  */
 
 /* Unlink a temporary file unless requested otherwise.  */
@@ -885,6 +888,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, compiler);
+  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
   obstack_ptr_grow (&argv_obstack, infile);
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-o");
@@ -962,11 +966,23 @@ main (int argc, char **argv)
      passed with @file.  Expand them into argv before processing.  */
   expandargv (&argc, &argv);
 
+  /* Find out whether we should compile binaries for i386 or x86-64.  */
+  for (int i = argc - 1; i > 0; i--)
+    if (strncmp (argv[i], "-foffload-abi=", sizeof ("-foffload-abi=") - 1) == 0)
+      {
+	if (strstr (argv[i], "ilp32"))
+	  target_ilp32 = true;
+	else if (!strstr (argv[i], "lp64"))
+	  fatal_error (input_location,
+		       "unrecognizable argument of option -foffload-abi");
+	break;
+      }
+
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, driver);
   obstack_ptr_grow (&argv_obstack, "-xlto");
-  obstack_ptr_grow (&argv_obstack, "-m64");
+  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
   obstack_ptr_grow (&argv_obstack, "-S");
 
   for (int ix = 1; ix != argc; ix++)


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

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

[Thomas Schwinge]

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

Hi!

OK for gcc-5-branch?

On Wed, 29 Apr 2015 18:26:06 +0200, I wrote:
> Committed in r222583:
> 
> commit df615909263269988fd9611f8d007902580829d9
> Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Wed Apr 29 16:23:26 2015 +0000
> 
>     [PR libgomp/65099] nvptx mkoffload: pass "-m32" or "-m64" to the compiler
>     
>     ... depending on "-foffload-abi=[...]".
>     
>     Coding style/code copied from gcc/config/i386/intelmic-mkoffload.c for
>     consistency.
>     
>     	gcc/
>     	* config/nvptx/mkoffload.c (target_ilp32): New variable.
>     	(main): Set it depending on "-foffload-abi=[...]".
>     	(compile_native, main): Use it to pass "-m32" or "-m64" to the
>     	compiler.
>     
>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@222583 138bc75d-0d04-0410-961f-82ee72b054a4
> ---
>  gcc/ChangeLog                |    8 ++++++++
>  gcc/config/nvptx/mkoffload.c |   18 +++++++++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git gcc/ChangeLog gcc/ChangeLog
> index aaa06c3..d7455e4 100644
> --- gcc/ChangeLog
> +++ gcc/ChangeLog
> @@ -1,3 +1,11 @@
> +2015-04-29  Thomas Schwinge  <thomas@codesourcery.com>
> +
> +	PR libgomp/65099
> +	* config/nvptx/mkoffload.c (target_ilp32): New variable.
> +	(main): Set it depending on "-foffload-abi=[...]".
> +	(compile_native, main): Use it to pass "-m32" or "-m64" to the
> +	compiler.
> +
>  2015-04-29  Alan Lawrence  <alan.lawrence@arm.com>
>  
>  	PR target/65770
> diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
> index dbc68bc..8687154 100644
> --- gcc/config/nvptx/mkoffload.c
> +++ gcc/config/nvptx/mkoffload.c
> @@ -126,6 +126,9 @@ static id_map *var_ids, **vars_tail = &var_ids;
>  static const char *ptx_name;
>  static const char *ptx_cfile_name;
>  
> +/* Shows if we should compile binaries for i386 instead of x86-64.  */
> +bool target_ilp32 = false;
> +
>  /* Delete tempfiles.  */
>  
>  /* Unlink a temporary file unless requested otherwise.  */
> @@ -885,6 +888,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
>    struct obstack argv_obstack;
>    obstack_init (&argv_obstack);
>    obstack_ptr_grow (&argv_obstack, compiler);
> +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
>    obstack_ptr_grow (&argv_obstack, infile);
>    obstack_ptr_grow (&argv_obstack, "-c");
>    obstack_ptr_grow (&argv_obstack, "-o");
> @@ -962,11 +966,23 @@ main (int argc, char **argv)
>       passed with @file.  Expand them into argv before processing.  */
>    expandargv (&argc, &argv);
>  
> +  /* Find out whether we should compile binaries for i386 or x86-64.  */
> +  for (int i = argc - 1; i > 0; i--)
> +    if (strncmp (argv[i], "-foffload-abi=", sizeof ("-foffload-abi=") - 1) == 0)
> +      {
> +	if (strstr (argv[i], "ilp32"))
> +	  target_ilp32 = true;
> +	else if (!strstr (argv[i], "lp64"))
> +	  fatal_error (input_location,
> +		       "unrecognizable argument of option -foffload-abi");
> +	break;
> +      }
> +
>    struct obstack argv_obstack;
>    obstack_init (&argv_obstack);
>    obstack_ptr_grow (&argv_obstack, driver);
>    obstack_ptr_grow (&argv_obstack, "-xlto");
> -  obstack_ptr_grow (&argv_obstack, "-m64");
> +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
>    obstack_ptr_grow (&argv_obstack, "-S");
>  
>    for (int ix = 1; ix != argc; ix++)


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

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

[Richard Biener]

On July 14, 2015 10:03:32 PM GMT+02:00, Thomas Schwinge <thomas@codesourcery.com> wrote:
>Hi!
>
>OK for gcc-5-branch?

OK

Richard

>On Wed, 29 Apr 2015 18:26:06 +0200, I wrote:
>> Committed in r222583:
>> 
>> commit df615909263269988fd9611f8d007902580829d9
>> Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
>> Date:   Wed Apr 29 16:23:26 2015 +0000
>> 
>>     [PR libgomp/65099] nvptx mkoffload: pass "-m32" or "-m64" to the
>compiler
>>     
>>     ... depending on "-foffload-abi=[...]".
>>     
>>     Coding style/code copied from
>gcc/config/i386/intelmic-mkoffload.c for
>>     consistency.
>>     
>>     	gcc/
>>     	* config/nvptx/mkoffload.c (target_ilp32): New variable.
>>     	(main): Set it depending on "-foffload-abi=[...]".
>>     	(compile_native, main): Use it to pass "-m32" or "-m64" to the
>>     	compiler.
>>     
>>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@222583
>138bc75d-0d04-0410-961f-82ee72b054a4
>> ---
>>  gcc/ChangeLog                |    8 ++++++++
>>  gcc/config/nvptx/mkoffload.c |   18 +++++++++++++++++-
>>  2 files changed, 25 insertions(+), 1 deletion(-)
>> 
>> diff --git gcc/ChangeLog gcc/ChangeLog
>> index aaa06c3..d7455e4 100644
>> --- gcc/ChangeLog
>> +++ gcc/ChangeLog
>> @@ -1,3 +1,11 @@
>> +2015-04-29  Thomas Schwinge  <thomas@codesourcery.com>
>> +
>> +	PR libgomp/65099
>> +	* config/nvptx/mkoffload.c (target_ilp32): New variable.
>> +	(main): Set it depending on "-foffload-abi=[...]".
>> +	(compile_native, main): Use it to pass "-m32" or "-m64" to the
>> +	compiler.
>> +
>>  2015-04-29  Alan Lawrence  <alan.lawrence@arm.com>
>>  
>>  	PR target/65770
>> diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
>> index dbc68bc..8687154 100644
>> --- gcc/config/nvptx/mkoffload.c
>> +++ gcc/config/nvptx/mkoffload.c
>> @@ -126,6 +126,9 @@ static id_map *var_ids, **vars_tail = &var_ids;
>>  static const char *ptx_name;
>>  static const char *ptx_cfile_name;
>>  
>> +/* Shows if we should compile binaries for i386 instead of x86-64. 
>*/
>> +bool target_ilp32 = false;
>> +
>>  /* Delete tempfiles.  */
>>  
>>  /* Unlink a temporary file unless requested otherwise.  */
>> @@ -885,6 +888,7 @@ compile_native (const char *infile, const char
>*outfile, const char *compiler)
>>    struct obstack argv_obstack;
>>    obstack_init (&argv_obstack);
>>    obstack_ptr_grow (&argv_obstack, compiler);
>> +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
>>    obstack_ptr_grow (&argv_obstack, infile);
>>    obstack_ptr_grow (&argv_obstack, "-c");
>>    obstack_ptr_grow (&argv_obstack, "-o");
>> @@ -962,11 +966,23 @@ main (int argc, char **argv)
>>       passed with @file.  Expand them into argv before processing. 
>*/
>>    expandargv (&argc, &argv);
>>  
>> +  /* Find out whether we should compile binaries for i386 or x86-64.
> */
>> +  for (int i = argc - 1; i > 0; i--)
>> +    if (strncmp (argv[i], "-foffload-abi=", sizeof
>("-foffload-abi=") - 1) == 0)
>> +      {
>> +	if (strstr (argv[i], "ilp32"))
>> +	  target_ilp32 = true;
>> +	else if (!strstr (argv[i], "lp64"))
>> +	  fatal_error (input_location,
>> +		       "unrecognizable argument of option -foffload-abi");
>> +	break;
>> +      }
>> +
>>    struct obstack argv_obstack;
>>    obstack_init (&argv_obstack);
>>    obstack_ptr_grow (&argv_obstack, driver);
>>    obstack_ptr_grow (&argv_obstack, "-xlto");
>> -  obstack_ptr_grow (&argv_obstack, "-m64");
>> +  obstack_ptr_grow (&argv_obstack, target_ilp32 ? "-m32" : "-m64");
>>    obstack_ptr_grow (&argv_obstack, "-S");
>>  
>>    for (int ix = 1; ix != argc; ix++)
>
>
>Grüße,
> Thomas

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

[Thomas Schwinge]

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

Hi!

On Sat, 11 Oct 2014 18:49:00 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> This is the last common infrastructure patch in the series.

> 1. -foffload=<targets>=<options>
>    By default, GCC will build offload images for all offload targets specified
> in configure, with non-target-specific options passed to host compiler.
> This option is used to control offload targets and options for them.
> 
> It can be used in a few ways:
> * -foffload=disable
>   Tells GCC to disable offload support.
>   OpenMP target regions will be run in host fallback mode.
> * -foffload=<targets>
>   Tells GCC to build offload images for <targets>.
>   They will be built with non-target-specific options passed to host compiler.
> * -foffload=<options>
>   Tells GCC to build offload images for all targets specified in configure. 
>   They will be built with non-target-specific options passed to host compiler
>   plus <options>.
> * -foffload=<targets>=<options>
>   Tells GCC to build offload images for <targets>.
>   They will be built with non-target-specific options passed to host compiler
>   plus <options>.
> 
> Options specified by -foffload are appended to the end of option set, so in case
> of option conflicts they have more priority.

(<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=[...]?

Given the code in gcc/configure.ac:

       939  for tgt in `echo $enable_offload_targets | sed 's/,/ /g'`; do
       940    tgt=`echo $tgt | sed 's/=.*//'`
       941    if test x"$offload_targets" = x; then
       942      offload_targets=$tgt
       943    else
       944      offload_targets="$offload_targets:$tgt"
       945    fi
       946  done
       947  AC_DEFINE_UNQUOTED(OFFLOAD_TARGETS, "$offload_targets",
       948    [Define to hold the list of target names suitable for offloading.])

..., we constuct OFFLOAD_TARGETS with the offload targets delimited by
colons, but when parsing -foffload options:

> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c

> +/* Parse -foffload option argument.  */
> +
> +static void
> +handle_foffload_option (const char *arg)
> +{
> +  [...]

..., we look for comma delimiters in the configure-time OFFLOAD_TARGETS:

      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?

> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c

> +/* Extract options for TARGET offload compiler from OPTIONS and append
> +   them to ARGV_OBSTACK.  */
> +
> +static void
> +append_offload_options (obstack *argv_obstack, const char *target,
> +			struct cl_decoded_option *options,
> +			unsigned int options_count)
> +{
> +  [...]

       571  /* Extract options for TARGET offload compiler from OPTIONS and append
       572     them to ARGV_OBSTACK.  */
       573  
       574  static void
       575  append_offload_options (obstack *argv_obstack, const char *target,
       576                          struct cl_decoded_option *options,
       577                          unsigned int options_count)
       578  {
       579    for (unsigned i = 0; i < options_count; i++)
       580      {
       581        const char *cur, *next, *opts;
       582        char **argv;
       583        unsigned argc;
       584        struct cl_decoded_option *option = &options[i];
       585  
       586        if (option->opt_index != OPT_foffload_)
       587          continue;
       588  
       589        /* If option argument starts with '-' then no target is specified.  That
       590           means offload options are specified for all targets, so we need to
       591           append them.  */
       592        if (option->arg[0] == '-')
       593          opts = option->arg;
       594        else
       595          {
       596            opts = strchr (option->arg, '=');
       597            if (!opts)
       598              continue;
       599  
       600            cur = option->arg;
       601  
       602            while (cur < opts)
       603              {
       604                next = strchr (cur, ',');
       605                if (next == NULL)
       606                  next = opts;
       607                next = (next > opts) ? opts : next;
       608  
       609                if (strlen (target) == (size_t) (next - cur)
       610                    && strncmp (target, cur, next - cur) == 0)
       611                  break;
       612  
       613                cur = next + 1;
       614              }
       615  
       616            if (cur >= opts)
       617              continue;
       618  
       619            opts++;
       620          }
       621  
       622        argv = buildargv (opts);
       623        for (argc = 0; argv[argc]; argc++)
       624          obstack_ptr_grow (argv_obstack, argv[argc]);
       625      }
       626  }

In this case, the other use of OFFLOAD_TARGETS in gcc/gcc.c:

      7570  /* Set up to remember the names of offload targets.  */
      7571  
      7572  void
      7573  driver::maybe_putenv_OFFLOAD_TARGETS () const
      7574  {
      7575    const char *targets = offload_targets;
      7576  
      7577    /* If no targets specified by -foffload, use all available targets.  */
      7578    if (!targets)
      7579      targets = OFFLOAD_TARGETS;
      7580  
      7581    if (strlen (targets) > 0)
      7582      {
      7583        obstack_grow (&collect_obstack, "OFFLOAD_TARGET_NAMES=",
      7584                      sizeof ("OFFLOAD_TARGET_NAMES=") - 1);
      7585        obstack_grow (&collect_obstack, targets,
      7586                      strlen (targets) + 1);
      7587        xputenv (XOBFINISH (&collect_obstack, char *));
      7588      }
      7589  
      7590    free (offload_targets);
      7591  }

... also isn't correct, because the environment variable
OFFLOAD_TARGET_NAMES in contrast is meant to separate the targets with
colons instead of commas (parsing code in
gcc/lto-wrapper.c:parse_env_var), so we can't just set the environment
variable OFFLOAD_TARGET_NAMES to OFFLOAD_TARGETS here.  The following
patch should address this (but I have not yet tested it):

diff --git gcc/gcc.c gcc/gcc.c
index 757bfc9..ad210c5a5 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,11 @@ process_command (unsigned int decoded_options_count,
 			   CL_DRIVER, &handlers, global_dc);
     }
 
+  /* If the user didn't specify any, default to all configured offload
+     targets.  */
+  if (offload_targets == NULL)
+    handle_foffload_option (OFFLOAD_TARGETS);
+
   if (output_file
       && strcmp (output_file, "-") != 0
       && strcmp (output_file, HOST_BIT_BUCKET) != 0)
@@ -7572,22 +7578,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.  */


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

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

[Ilya Verbin]

2015-09-21 18:15 GMT+03:00 Thomas Schwinge <thomas@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.

  -- Ilya

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

[Thomas Schwinge]

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

Hi!

On Mon, 21 Sep 2015 19:41:59 +0300, Ilya Verbin <iverbin@gmail.com> wrote:
> 2015-09-21 18:15 GMT+03:00 Thomas Schwinge <thomas@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 <thomas@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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

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

[Bernd Schmidt]

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 except this double ChangeLog seems messed up.


Bernd

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

[Thomas Schwinge]

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

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(-)

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);
 	    }
 	}
 


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

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

[Ilya Verbin]

On Wed, Dec 10, 2014 at 01:48:21 +0300, Ilya Verbin wrote:
> On 09 Dec 14:59, Richard Biener wrote:
> > On Mon, 8 Dec 2014, Ilya Verbin wrote:
> > > Unfortunately, this fix was not general enough.
> > > There might be cases when mixed object files get into lto-wrapper, ie some of
> > > them contain only LTO sections, some contain only offload sections, and some
> > > contain both.  But when lto-wrapper will pass all these files to recompilation,
> > > the compiler might crash (it depends on the order of input files), since in
> > > read_cgraph_and_symbols it expects that *all* input files contain IR section of
> > > given type.
> > > This patch splits input objects from argv into lto_argv and offload_argv, so
> > > that all files in arrays contain corresponding IR.
> > > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload
> > > IR without LTO, to claimed_files, since this may corrupt a resolution file.
> > > 
> > > Tested on various combinations of files with/without -flto and with/without
> > > offload, using trunk ld and gold, also tested on ld without plugin support.
> > > Bootstrap and make check passed on x86_64-linux and i686-linux.  Ok for trunk?
> > 
> > Did you check that bootstrap-lto still works?  Ok if so.
> 
> Yes, bootstrap-lto passed.
> Committed revision 218543.

I don't know how I missed this a year ago, but mixing of LTO objects with
offloading-without-LTO objects still doesn't work :(
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68463 filed about that.
Any thoughts how to fix this?

Thanks,
  -- Ilya

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

[Richard Biener]

On Fri, 20 Nov 2015, Ilya Verbin wrote:

> On Wed, Dec 10, 2014 at 01:48:21 +0300, Ilya Verbin wrote:
> > On 09 Dec 14:59, Richard Biener wrote:
> > > On Mon, 8 Dec 2014, Ilya Verbin wrote:
> > > > Unfortunately, this fix was not general enough.
> > > > There might be cases when mixed object files get into lto-wrapper, ie some of
> > > > them contain only LTO sections, some contain only offload sections, and some
> > > > contain both.  But when lto-wrapper will pass all these files to recompilation,
> > > > the compiler might crash (it depends on the order of input files), since in
> > > > read_cgraph_and_symbols it expects that *all* input files contain IR section of
> > > > given type.
> > > > This patch splits input objects from argv into lto_argv and offload_argv, so
> > > > that all files in arrays contain corresponding IR.
> > > > Similarly, in lto-plugin, it was bad idea to add objects, which contain offload
> > > > IR without LTO, to claimed_files, since this may corrupt a resolution file.
> > > > 
> > > > Tested on various combinations of files with/without -flto and with/without
> > > > offload, using trunk ld and gold, also tested on ld without plugin support.
> > > > Bootstrap and make check passed on x86_64-linux and i686-linux.  Ok for trunk?
> > > 
> > > Did you check that bootstrap-lto still works?  Ok if so.
> > 
> > Yes, bootstrap-lto passed.
> > Committed revision 218543.
> 
> I don't know how I missed this a year ago, but mixing of LTO objects with
> offloading-without-LTO objects still doesn't work :(
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68463 filed about that.
> Any thoughts how to fix this?

Don't claim files you don't handle.

Richard.