Re: [PATCH 4/4] OpenMP 4.0 offloading to Intel MIC: non-fallback testing

[Ilya Verbin <iverbin at gmail dot com>]

On 28 Jan 19:20, Ilya Verbin wrote:
> On 28 Jan 17:15, Jakub Jelinek wrote:
> > On Wed, Jan 28, 2015 at 07:02:59PM +0300, Ilya Verbin wrote:
> > > +	= XNEWVEC (char, len + sizeof ("-B" "../" DEFAULT_TARGET_MACHINE
> > > +				       "/libgomp/"));
> > > +      sprintf (optional_target_path2, "-B%s/../../../" DEFAULT_TARGET_MACHINE
> > > +				      "/libgomp/", current_path);
> > 
> > This will surely overflow the buffer, won't it?  There is space just for
> > "../" but you put there "/../../../".
> > 
> > I'd strongly prefer if you rewrote all these XNEWVEC or XRESIZEVEC etc.
> > + sprintf cases into concat, like
> >   optional_target_path2 = concat ("-B", current_path,
> > 				  "/../../../" DEFAULT_TARGET_MACHINE
> > 				  "/libgomp/", NULL);
> > and similar.  That way you avoid all such bugs.
> 
> The variable 'len' contains sizeof ("/../../").
> I agree that this code looks ugly :)  I'll rewrite it using concat.

Here is the patch with concat.


diff --git a/gcc/config.gcc b/gcc/config.gcc
index abd915e..0ebdbd2 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4374,7 +4374,7 @@ fi
 case ${target} in
 i[34567]86-*-* | x86_64-*-*)
 	if test x$enable_as_accelerator = xyes; then
-		extra_programs="mkoffload\$(exeext)"
+		extra_programs="mkoffload\$(exeext) accel/${target_noncanonical}/mkoffload$(exeext)"
 	fi
 	;;
 esac
diff --git a/gcc/config/i386/intelmic-mkoffload.c b/gcc/config/i386/intelmic-mkoffload.c
index edc3f92e..bc71004 100644
--- a/gcc/config/i386/intelmic-mkoffload.c
+++ b/gcc/config/i386/intelmic-mkoffload.c
@@ -22,13 +22,13 @@
 
 #include "config.h"
 #include <libgen.h>
-#include "libgomp-plugin.h"
 #include "system.h"
 #include "coretypes.h"
 #include "obstack.h"
 #include "intl.h"
 #include "diagnostic.h"
 #include "collect-utils.h"
+#include "intelmic-offload.h"
 
 const char tool_name[] = "intelmic mkoffload";
 
@@ -45,6 +45,13 @@ const char *temp_files[MAX_NUM_TEMPS];
 /* Shows if we should compile binaries for i386 instead of x86-64.  */
 bool target_ilp32 = false;
 
+/* Optional prefixes for the target compiler, which are required when target
+   compiler is not installed.  */
+char *optional_target_path1 = NULL;
+char *optional_target_path2 = NULL;
+char *optional_target_lib_path = NULL;
+
+
 /* Delete tempfiles and exit function.  */
 void
 tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
@@ -151,14 +158,17 @@ access_check (const char *name, int mode)
   return access (name, mode);
 }
 
-/* Find target compiler using a path from COLLECT_GCC or COMPILER_PATH.  */
+/* Find target compiler using a path from COLLECT_GCC, COMPILER_PATH, or a path
+   relative to ARGV0.  */
 static char *
-find_target_compiler (const char *name)
+find_target_compiler (const char *argv0)
 {
   bool found = false;
   char **paths = NULL;
   unsigned n_paths, i;
+  const char *current_path;
   const char *collect_path = dirname (ASTRDUP (getenv ("COLLECT_GCC")));
+  const char *name = GCC_INSTALL_NAME;
   size_t len = strlen (collect_path) + 1 + strlen (name) + 1;
   char *target_compiler = XNEWVEC (char, len);
   sprintf (target_compiler, "%s/%s", collect_path, name);
@@ -177,13 +187,32 @@ find_target_compiler (const char *name)
       if (access_check (target_compiler, X_OK) == 0)
 	{
 	  found = true;
-	  break;
+	  goto out;
 	}
     }
+  XDELETEVEC (target_compiler);
+
+  /* If installed compiler wasn't found, try to find a non-installed compiler,
+     using a path relative to mkoffload.  */
+  current_path = dirname (ASTRDUP (argv0));
+  target_compiler = concat (current_path, "/../../xgcc", NULL);
+  if (access_check (target_compiler, X_OK) == 0)
+    {
+      optional_target_path1 = concat ("-B", current_path, "/../../", NULL);
+      optional_target_path2
+	= concat ("-B", current_path,
+		  "/../../../" DEFAULT_TARGET_MACHINE "/libgomp/", NULL);
+      optional_target_lib_path
+	= concat ("-L", current_path,
+		  "/../../../" DEFAULT_TARGET_MACHINE "/libgomp/.libs/", NULL);
+      found = true;
+    }
 
 out:
   free_array_of_ptrs ((void **) paths, n_paths);
-  return found ? target_compiler : NULL;
+  if (!found)
+    fatal_error ("offload compiler %s not found", name);
+  return target_compiler;
 }
 
 static void
@@ -193,6 +222,14 @@ compile_for_target (struct obstack *argv_obstack)
     obstack_ptr_grow (argv_obstack, "-m32");
   else
     obstack_ptr_grow (argv_obstack, "-m64");
+
+  if (optional_target_path1)
+    obstack_ptr_grow (argv_obstack, optional_target_path1);
+  if (optional_target_path2)
+    obstack_ptr_grow (argv_obstack, optional_target_path2);
+  if (optional_target_lib_path)
+    obstack_ptr_grow (argv_obstack, optional_target_lib_path);
+
   obstack_ptr_grow (argv_obstack, NULL);
   char **argv = XOBFINISH (argv_obstack, char **);
 
@@ -346,7 +383,7 @@ generate_host_descr_file (const char *host_compiler)
 	   "init (void)\n"
 	   "{\n"
 	   "  GOMP_offload_register (&__OFFLOAD_TABLE__, %d, __offload_target_data);\n"
-	   "}\n", OFFLOAD_TARGET_TYPE_INTEL_MIC);
+	   "}\n", GOMP_DEVICE_INTEL_MIC);
   fclose (src_file);
 
   unsigned new_argc = 0;
@@ -483,11 +520,7 @@ main (int argc, char **argv)
   if (!host_compiler)
     fatal_error ("COLLECT_GCC must be set");
 
-  const char *target_driver_name
-    = DEFAULT_REAL_TARGET_MACHINE "-accel-" DEFAULT_TARGET_MACHINE "-gcc";
-  char *target_compiler = find_target_compiler (target_driver_name);
-  if (target_compiler == NULL)
-    fatal_error ("offload compiler %s not found", target_driver_name);
+  char *target_compiler = find_target_compiler (argv[0]);
 
   /* We may be called with all the arguments stored in some file and
      passed with @file.  Expand them into argv before processing.  */
diff --git a/gcc/config/i386/t-intelmic b/gcc/config/i386/t-intelmic
index 8b36e0d..86e10eb 100644
--- a/gcc/config/i386/t-intelmic
+++ b/gcc/config/i386/t-intelmic
@@ -1,9 +1,16 @@
+CFLAGS-mkoffload.o += $(DRIVER_DEFINES) \
+	-DGCC_INSTALL_NAME=\"$(GCC_INSTALL_NAME)\"
+
 mkoffload.o: $(srcdir)/config/i386/intelmic-mkoffload.c | insn-modes.h
-	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
-	  -I$(srcdir)/../libgomp \
-	  -DDEFAULT_REAL_TARGET_MACHINE=\"$(real_target_noncanonical)\" \
-	  -DDEFAULT_TARGET_MACHINE=\"$(target_noncanonical)\" \
-	  $< $(OUTPUT_OPTION)
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 mkoffload$(exeext): mkoffload.o collect-utils.o libcommon-target.a $(LIBIBERTY) $(LIBDEPS)
-	$(COMPILER) -o $@ mkoffload.o collect-utils.o libcommon-target.a $(LIBIBERTY) $(LIBS)
+	$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	  mkoffload.o collect-utils.o libcommon-target.a $(LIBIBERTY) $(LIBS)
+
+accel/$(target_noncanonical)/mkoffload$(exeext): mkoffload$(exeext)
+	test -d accel || mkdir accel
+	test -d accel/$(target_noncanonical) || mkdir accel/$(target_noncanonical)
+	rm -f $@
+	$(LN) mkoffload$(exeext) $@
diff --git a/libgomp/configure b/libgomp/configure
index 0818707..8d96950 100755
--- a/libgomp/configure
+++ b/libgomp/configure
@@ -15226,6 +15226,9 @@ if test x"$enable_offload_targets" != x; then
     case $tgt in
       *-intelmic-* | *-intelmicemul-*)
 	tgt_name=intelmic
+	if test x"$tgt_dir" != x; then
+	  offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/$tgt/liboffloadmic/.libs"
+	fi
 	;;
       nvptx*)
         tgt_name=nvptx
@@ -15276,7 +15279,7 @@ rm -f core conftest.err conftest.$ac_objext \
       offload_targets=$offload_targets,$tgt_name
     fi
     if test x"$tgt_dir" != x; then
-      offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
+      offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/gcc -B$tgt_dir/bin"
       offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/lib64:$tgt_dir/lib:$tgt_dir/lib32"
     else
       offload_additional_options="$offload_additional_options -B\$(libexecdir)/gcc/\$(target_alias)/\$(gcc_version) -B\$(bindir)"
diff --git a/libgomp/plugin/configfrag.ac b/libgomp/plugin/configfrag.ac
index 254c688..2861e68 100644
--- a/libgomp/plugin/configfrag.ac
+++ b/libgomp/plugin/configfrag.ac
@@ -94,6 +94,9 @@ if test x"$enable_offload_targets" != x; then
     case $tgt in
       *-intelmic-* | *-intelmicemul-*)
 	tgt_name=intelmic
+	if test x"$tgt_dir" != x; then
+	  offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/$tgt/liboffloadmic/.libs"
+	fi
 	;;
       nvptx*)
         tgt_name=nvptx
@@ -133,7 +136,7 @@ if test x"$enable_offload_targets" != x; then
       offload_targets=$offload_targets,$tgt_name
     fi
     if test x"$tgt_dir" != x; then
-      offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
+      offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/gcc -B$tgt_dir/bin"
       offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/lib64:$tgt_dir/lib:$tgt_dir/lib32"
     else
       offload_additional_options="$offload_additional_options -B\$(libexecdir)/gcc/\$(target_alias)/\$(gcc_version) -B\$(bindir)"


  -- Ilya