Re: [PATCH][OpenMP] Forbid usage of non-target functions in target regions

[Ilya Verbin <iverbin at gmail dot com>]

On Mon, Feb 02, 2015 at 13:10:19 +0100, Jakub Jelinek wrote:
> [...]  Generally, the solution if something goes
> wrong during the offloading compilation should be just to give up on the
> offloading to the particular offloading target (i.e. fill in the sections
> libgomp reads in a way that will result in host fallback).

Do you mean something like this?
Bootstrapped/regtested on x86_64-linux and i686-linux.
Is this patch OK for stage4 or stage1?


gcc/
	* collect-utils.c (do_wait, fork_execute): New argument.
	* collect-utils.h: Likewise.
	* collect2.c: Pass new argument to do_wait and fork_execute.
	* config/i386/intelmic-mkoffload.c: Likewise.
	(compile_for_target): Don't call fatal_error if compilation failed.
	(generate_target_descr_file, generate_target_offloadend_file)
	(prepare_target_image): Pass out_filename to compile_for_target.
	* config/nvptx/mkoffload.c: Pass new argument fork_execute.
	* lto-wrapper.c (num_offload_targets): New static global variable.
	(compile_offload_image): Return NULL if an image was not created.
	(compile_images_for_offload_targets): Call warning instead of
	fatal_error if an image was not created.
	(run_gcc): Do not return empty images to linker.
	Pass new argument to do_wait and fork_execute.
libgomp/
	* target.c (gomp_target_fallback): New static function.
	(GOMP_target): Move host fallback to the new gomp_target_fallback.
	Run gomp_target_fallback if tgt_fn is not present in the splay tree.


diff --git a/gcc/collect-utils.c b/gcc/collect-utils.c
index 6bbe9eb..2dce3c9 100644
--- a/gcc/collect-utils.c
+++ b/gcc/collect-utils.c
@@ -85,10 +85,10 @@ collect_wait (const char *prog, struct pex_obj *pex)
 }
 
 void
-do_wait (const char *prog, struct pex_obj *pex)
+do_wait (const char *prog, struct pex_obj *pex, bool non_fatal)
 {
   int ret = collect_wait (prog, pex);
-  if (ret != 0)
+  if (!non_fatal && ret != 0)
     fatal_error (input_location, "%s returned %d exit status", prog, ret);
 
   if (response_file && !save_temps)
@@ -201,13 +201,13 @@ collect_execute (const char *prog, char **argv, const char *outname,
 }
 
 void
-fork_execute (const char *prog, char **argv, bool use_atfile)
+fork_execute (const char *prog, char **argv, bool use_atfile, bool non_fatal)
 {
   struct pex_obj *pex;
 
   pex = collect_execute (prog, argv, NULL, NULL,
 			 PEX_LAST | PEX_SEARCH, use_atfile);
-  do_wait (prog, pex);
+  do_wait (prog, pex, non_fatal);
 }
 
 /* Delete tempfiles.  */
diff --git a/gcc/collect-utils.h b/gcc/collect-utils.h
index 2b3ed44..7b3b3da 100644
--- a/gcc/collect-utils.h
+++ b/gcc/collect-utils.h
@@ -29,8 +29,8 @@ extern struct pex_obj *collect_execute (const char *, char **,
 					const char *, const char *,
 					int, bool);
 extern int collect_wait (const char *, struct pex_obj *);
-extern void do_wait (const char *, struct pex_obj *);
-extern void fork_execute (const char *, char **, bool);
+extern void do_wait (const char *, struct pex_obj *, bool);
+extern void fork_execute (const char *, char **, bool, bool);
 extern void utils_cleanup (bool);
 
 
diff --git a/gcc/collect2.c b/gcc/collect2.c
index b53e151..f8be7da 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -758,7 +758,7 @@ maybe_run_lto_and_relink (char **lto_ld_argv, char **object_lst,
 
 	obstack_free (&temporary_obstack, temporary_firstobj);
       }
-      do_wait (prog, pex);
+      do_wait (prog, pex, false);
       pex = NULL;
 
       /* Compute memory needed for new LD arguments.  At most number of original arguemtns
@@ -803,7 +803,8 @@ maybe_run_lto_and_relink (char **lto_ld_argv, char **object_lst,
 
       /* Run the linker again, this time replacing the object files
          optimized by the LTO with the temporary file generated by the LTO.  */
-      fork_execute ("ld", out_lto_ld_argv, HAVE_GNU_LD && at_file_supplied);
+      fork_execute ("ld", out_lto_ld_argv, HAVE_GNU_LD && at_file_supplied,
+		    false);
       post_ld_pass (true);
       free (lto_ld_argv);
 
@@ -813,7 +814,7 @@ maybe_run_lto_and_relink (char **lto_ld_argv, char **object_lst,
     {
       /* Our caller is relying on us to do the link
          even though there is no LTO back end work to be done.  */
-      fork_execute ("ld", lto_ld_argv, HAVE_GNU_LD && at_file_supplied);
+      fork_execute ("ld", lto_ld_argv, HAVE_GNU_LD && at_file_supplied, false);
       post_ld_pass (false);
     }
   else
@@ -1706,7 +1707,7 @@ main (int argc, char **argv)
 	  strip_argv[0] = strip_file_name;
 	  strip_argv[1] = output_file;
 	  strip_argv[2] = (char *) 0;
-	  fork_execute ("strip", real_strip_argv, false);
+	  fork_execute ("strip", real_strip_argv, false, false);
 	}
 
 #ifdef COLLECT_EXPORT_LIST
@@ -1792,7 +1793,7 @@ main (int argc, char **argv)
   /* Assemble the constructor and destructor tables.
      Link the tables in with the rest of the program.  */
 
-  fork_execute ("gcc",  c_argv, at_file_supplied);
+  fork_execute ("gcc",  c_argv, at_file_supplied, false);
 #ifdef COLLECT_EXPORT_LIST
   /* On AIX we must call tlink because of possible templates resolution.  */
   do_tlink (ld2_argv, object_lst);
@@ -1805,7 +1806,7 @@ main (int argc, char **argv)
     maybe_run_lto_and_relink (ld2_argv, object_lst, object, true);
   else
     {
-      fork_execute ("ld", ld2_argv, HAVE_GNU_LD && at_file_supplied);
+      fork_execute ("ld", ld2_argv, HAVE_GNU_LD && at_file_supplied, false);
       post_ld_pass (false);
     }
 
@@ -2486,7 +2487,7 @@ scan_prog_file (const char *prog_name, scanpass which_pass,
   if (debug)
     fprintf (stderr, "\n");
 
-  do_wait (nm_file_name, pex);
+  do_wait (nm_file_name, pex, false);
 
   signal (SIGINT,  int_handler);
 #ifdef SIGQUIT
@@ -2606,7 +2607,7 @@ scan_libraries (const char *prog_name)
   if (debug)
     fprintf (stderr, "\n");
 
-  do_wait (ldd_file_name, pex);
+  do_wait (ldd_file_name, pex, false);
 
   signal (SIGINT,  int_handler);
 #ifdef SIGQUIT
@@ -3023,7 +3024,7 @@ do_dsymutil (const char *output_file) {
 
   pex = collect_execute (dsymutil, real_argv, NULL, NULL,
 			 PEX_LAST | PEX_SEARCH, false);
-  do_wait (dsymutil, pex);
+  do_wait (dsymutil, pex, false);
 }
 
 static void
diff --git a/gcc/config/i386/intelmic-mkoffload.c b/gcc/config/i386/intelmic-mkoffload.c
index e6394e9..45d8677 100644
--- a/gcc/config/i386/intelmic-mkoffload.c
+++ b/gcc/config/i386/intelmic-mkoffload.c
@@ -187,12 +187,15 @@ out:
 }
 
 static void
-compile_for_target (struct obstack *argv_obstack)
+compile_for_target (struct obstack *argv_obstack, const char *out_filename)
 {
   if (target_ilp32)
     obstack_ptr_grow (argv_obstack, "-m32");
   else
     obstack_ptr_grow (argv_obstack, "-m64");
+
+  obstack_ptr_grow (argv_obstack, "-o");
+  obstack_ptr_grow (argv_obstack, out_filename);
   obstack_ptr_grow (argv_obstack, NULL);
   char **argv = XOBFINISH (argv_obstack, char **);
 
@@ -206,7 +209,7 @@ compile_for_target (struct obstack *argv_obstack)
   unsetenv ("LIBRARY_PATH");
   unsetenv ("LD_RUN_PATH");
 
-  fork_execute (argv[0], argv, false);
+  fork_execute (argv[0], argv, false, true);
   obstack_free (argv_obstack, NULL);
 
   /* Restore environment variables.  */
@@ -214,6 +217,12 @@ compile_for_target (struct obstack *argv_obstack)
   xputenv (concat ("COMPILER_PATH=", cpath, NULL));
   xputenv (concat ("LIBRARY_PATH=", lpath, NULL));
   xputenv (concat ("LD_RUN_PATH=", rpath, NULL));
+
+  /* If something went wrong during the compilation, exit mkoffload without
+     producing an object file.  */
+  struct stat st;
+  if (stat (out_filename, &st) != 0 || st.st_size == 0)
+    exit (EXIT_SUCCESS);
 }
 
 /* Generates object file with the descriptor for the target library.  */
@@ -268,9 +277,7 @@ generate_target_descr_file (const char *target_compiler)
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
   obstack_ptr_grow (&argv_obstack, src_filename);
-  obstack_ptr_grow (&argv_obstack, "-o");
-  obstack_ptr_grow (&argv_obstack, obj_filename);
-  compile_for_target (&argv_obstack);
+  compile_for_target (&argv_obstack, obj_filename);
 
   return obj_filename;
 }
@@ -306,9 +313,7 @@ generate_target_offloadend_file (const char *target_compiler)
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
   obstack_ptr_grow (&argv_obstack, src_filename);
-  obstack_ptr_grow (&argv_obstack, "-o");
-  obstack_ptr_grow (&argv_obstack, obj_filename);
-  compile_for_target (&argv_obstack);
+  compile_for_target (&argv_obstack, obj_filename);
 
   return obj_filename;
 }
@@ -364,7 +369,7 @@ generate_host_descr_file (const char *host_compiler)
   new_argv[new_argc++] = obj_filename;
   new_argv[new_argc++] = NULL;
 
-  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false);
+  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false, false);
 
   return obj_filename;
 }
@@ -403,9 +408,7 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   if (!out_obj_filename)
     fatal_error (input_location, "output file not specified");
   obstack_ptr_grow (&argv_obstack, opt2);
-  obstack_ptr_grow (&argv_obstack, "-o");
-  obstack_ptr_grow (&argv_obstack, target_so_filename);
-  compile_for_target (&argv_obstack);
+  compile_for_target (&argv_obstack, target_so_filename);
 
   /* Run objcopy.  */
   char *rename_section_opt
@@ -426,7 +429,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   objcopy_argv[8] = "--rename-section";
   objcopy_argv[9] = rename_section_opt;
   objcopy_argv[10] = NULL;
-  fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false);
+  fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false,
+		false);
 
   /* Objcopy has created symbols, containing the input file name with
      special characters replaced with '_'.  We are going to rename these
@@ -464,7 +468,7 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   objcopy_argv[6] = "--redefine-sym";
   objcopy_argv[7] = opt_for_objcopy[2];
   objcopy_argv[8] = NULL;
-  fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false);
+  fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false, false);
 
   return target_so_filename;
 }
@@ -527,7 +531,7 @@ main (int argc, char **argv)
   new_argv[new_argc++] = "-o";
   new_argv[new_argc++] = out_obj_filename;
   new_argv[new_argc++] = NULL;
-  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false);
+  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false, false);
 
   /* Run objcopy on the resultant object file to localize generated symbols
      to avoid conflicting between different DSO and an executable.  */
@@ -540,7 +544,7 @@ main (int argc, char **argv)
   new_argv[6] = symbols[2];
   new_argv[7] = out_obj_filename;
   new_argv[8] = NULL;
-  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false);
+  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false, false);
 
   return 0;
 }
diff --git a/gcc/config/nvptx/mkoffload.c b/gcc/config/nvptx/mkoffload.c
index 739aee8..67c87b9 100644
--- a/gcc/config/nvptx/mkoffload.c
+++ b/gcc/config/nvptx/mkoffload.c
@@ -817,7 +817,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   obstack_ptr_grow (&argv_obstack, NULL);
 
   const char **new_argv = XOBFINISH (&argv_obstack, const char **);
-  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true);
+  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true, false);
   obstack_free (&argv_obstack, NULL);
 }
 
@@ -884,7 +884,7 @@ main (int argc, char **argv)
   unsetenv ("COMPILER_PATH");
   unsetenv ("LIBRARY_PATH");
 
-  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true);
+  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true, false);
   obstack_free (&argv_obstack, NULL);
 
   xputenv (concat ("GCC_EXEC_PREFIX=", execpath, NULL));
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 404cb68..aef077f 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -68,6 +68,7 @@ static unsigned int nr;
 static char **input_names;
 static char **output_names;
 static char **offload_names;
+static unsigned num_offload_targets;
 static const char *offloadbegin, *offloadend;
 static char *makefile;
 
@@ -644,6 +645,7 @@ compile_offload_image (const char *target, const char *compiler_path,
 		       struct cl_decoded_option *linker_opts,
 		       unsigned int linker_opt_count)
 {
+  struct stat st;
   char *filename = NULL;
   char **argv;
   char *suffix
@@ -691,11 +693,18 @@ compile_offload_image (const char *target, const char *compiler_path,
 
       obstack_ptr_grow (&argv_obstack, NULL);
       argv = XOBFINISH (&argv_obstack, char **);
-      fork_execute (argv[0], argv, true);
+      fork_execute (argv[0], argv, true, false);
       obstack_free (&argv_obstack, NULL);
     }
 
   free_array_of_ptrs ((void **) paths, n_paths);
+
+  if (filename && (stat (filename, &st) != 0 || st.st_size == 0))
+    {
+      XDELETEVEC (filename);
+      filename = NULL;
+    }
+
   return filename;
 }
 
@@ -716,28 +725,27 @@ compile_images_for_offload_targets (unsigned in_argc, char *in_argv[],
   const char *target_names = getenv (OFFLOAD_TARGET_NAMES_ENV);
   if (!target_names)
     return;
-  unsigned num_targets = parse_env_var (target_names, &names, NULL);
+  num_offload_targets = parse_env_var (target_names, &names, NULL);
 
   const char *compiler_path = getenv ("COMPILER_PATH");
   if (!compiler_path)
     goto out;
 
   /* Prepare an image for each target and save the name of the resultant object
-     file to the OFFLOAD_NAMES array.  It is terminated by a NULL entry.  */
-  offload_names = XCNEWVEC (char *, num_targets + 1);
-  for (unsigned i = 0; i < num_targets; i++)
+     file to the OFFLOAD_NAMES array.  */
+  offload_names = XCNEWVEC (char *, num_offload_targets + 1);
+  for (unsigned i = 0; i < num_offload_targets; i++)
     {
       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 (input_location,
-		     "problem with building target image for %s\n", names[i]);
+	warning (0, "problem with building target image for %s", names[i]);
     }
 
  out:
-  free_array_of_ptrs ((void **) names, num_targets);
+  free_array_of_ptrs ((void **) names, num_offload_targets);
 }
 
 /* Copy a file from SRC to DEST.  */
@@ -1063,9 +1071,10 @@ run_gcc (unsigned argc, char *argv[])
       if (offload_names)
 	{
 	  find_offloadbeginend ();
-	  for (i = 0; offload_names[i]; i++)
-	    printf ("%s\n", offload_names[i]);
-	  free_array_of_ptrs ((void **) offload_names, i);
+	  for (i = 0; i < num_offload_targets; i++)
+	    if (offload_names[i])
+	      printf ("%s\n", offload_names[i]);
+	  free_array_of_ptrs ((void **) offload_names, num_offload_targets);
 	}
     }
 
@@ -1154,7 +1163,7 @@ run_gcc (unsigned argc, char *argv[])
 
   new_argv = XOBFINISH (&argv_obstack, const char **);
   argv_ptr = &new_argv[new_head_argc];
-  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true);
+  fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true, false);
 
   if (lto_mode == LTO_MODE_LTO)
     {
@@ -1263,7 +1272,7 @@ cont:
 	  else
 	    {
 	      fork_execute (new_argv[0], CONST_CAST (char **, new_argv),
-			    true);
+			    true, false);
 	      maybe_unlink (input_name);
 	    }
 
@@ -1301,7 +1310,7 @@ cont:
 	  new_argv[i++] = NULL;
 	  pex = collect_execute (new_argv[0], CONST_CAST (char **, new_argv),
 				 NULL, NULL, PEX_SEARCH, false);
-	  do_wait (new_argv[0], pex);
+	  do_wait (new_argv[0], pex, false);
 	  maybe_unlink (makefile);
 	  makefile = NULL;
 	  for (i = 0; i < nr; ++i)
diff --git a/libgomp/target.c b/libgomp/target.c
index 73e757a..4d770c4 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -735,6 +735,24 @@ gomp_fini_device (struct gomp_device_descr *devicep)
   devicep->is_initialized = false;
 }
 
+/* Host fallback for GOMP_target routine.  */
+
+static void
+gomp_target_fallback (void (*fn) (void *), void **hostaddrs)
+{
+  struct gomp_thread old_thr, *thr = gomp_thread ();
+  old_thr = *thr;
+  memset (thr, '\0', sizeof (*thr));
+  if (gomp_places_list)
+    {
+      thr->place = old_thr.place;
+      thr->ts.place_partition_len = gomp_places_list_len;
+    }
+  fn (hostaddrs);
+  gomp_free_thread (thr);
+  *thr = old_thr;
+}
+
 /* Called when encountering a target directive.  If DEVICE
    is GOMP_DEVICE_ICV, it means use device-var ICV.  If it is
    GOMP_DEVICE_HOST_FALLBACK (or any value
@@ -754,21 +772,7 @@ GOMP_target (int device, void (*fn) (void *), const void *unused,
 
   if (devicep == NULL
       || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400))
-    {
-      /* Host fallback.  */
-      struct gomp_thread old_thr, *thr = gomp_thread ();
-      old_thr = *thr;
-      memset (thr, '\0', sizeof (*thr));
-      if (gomp_places_list)
-	{
-	  thr->place = old_thr.place;
-	  thr->ts.place_partition_len = gomp_places_list_len;
-	}
-      fn (hostaddrs);
-      gomp_free_thread (thr);
-      *thr = old_thr;
-      return;
-    }
+    return gomp_target_fallback (fn, hostaddrs);
 
   gomp_mutex_lock (&devicep->lock);
   if (!devicep->is_initialized)
@@ -792,8 +796,11 @@ GOMP_target (int device, void (*fn) (void *), const void *unused,
       k.host_end = k.host_start + 1;
       splay_tree_key tgt_fn = splay_tree_lookup (&mm->splay_tree, &k);
       if (tgt_fn == NULL)
-	gomp_fatal ("Target function wasn't mapped");
-
+	{
+	  gomp_target_fallback (fn, hostaddrs);
+	  gomp_mutex_unlock (&mm->lock);
+	  return;
+	}
       gomp_mutex_unlock (&mm->lock);
 
       fn_addr = (void *) tgt_fn->tgt->tgt_start;


Thanks,
  -- Ilya