[PATCH 1/4] Add mkoffload for Intel MIC

[Jakub Jelinek]

On Tue, Oct 21, 2014 at 09:16:02PM +0400, Ilya Verbin wrote:
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3895,3 +3895,14 @@ DEPFILES = \
>    $(foreach obj,$(ALL_HOST_OBJS),\
>      $(dir $(obj))$(DEPDIR)/$(patsubst %.o,%.Po,$(notdir $(obj))))
>  -include $(DEPFILES)
> +
> +
> +mkoffload.o: $(srcdir)/config/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)
> +
> +mkoffload$(exeext): mkoffload.o collect-utils.o libcommon-target.a $(LIBIBERTY) $(LIBDEPS)
> +	$(COMPILER) -o $@ mkoffload.o collect-utils.o libcommon-target.a $(LIBIBERTY) $(LIBS)

I don't like this.  I thought every offloading target will have its own
mkoffload binary, so hardcoding a particular mkoffload implementation in
the generic Makefile.in looks very much wrong.  The rules should come from
some config/i386/t-intelmic target snippet enabled by config.gcc.

Also, is there a reason why do you want to use config/intelmic/
subdirectory?  Is there any plan to make intelmic offloading for different
ISA than x86_64?  I mean, even K10M would use i386/ backend, if it got
added, right?
So, I'd use config/i386/t-intelmic and config/i386/intelmic-mkoffload.c .
> +
> +/* Delete tempfiles and exit function.  */
> +void
> +tool_cleanup (__attribute__((unused)) bool from_signal)

boo from_signal ATTRIBUTE_UNUSED ?

> +  values = (char**) xmalloc (num * sizeof (char*));

Formatting, spaces before *, so char ** and char *.

> +  curval = str;
> +  nextval = strchrnul (curval, ':');
> +
> +  for (i = 0; i < num; i++)
> +    {
> +      int l = nextval - curval;
> +      values[i] = (char*) xmalloc (l + 1);

Idem.

> +  /* Objcopy has created symbols, containing the input file name with
> +     special characters replaced with '_'.  We are going to rename these
> +     new symbols.  */
> +  char *symbol_name = XALLOCAVEC (char, strlen (target_so_filename) + 1);
> +  int i = 0;
> +  while (target_so_filename[i])
> +    {
> +      char c = target_so_filename[i];
> +      if ((c == '/') || (c == '.'))
> +	c = '_';
> +      symbol_name[i] = c;
> +      i++;
> +    }
> +  symbol_name[i] = 0;

So, you certainly know strlen (symbol_name) here.

> +  char *opt_for_objcopy[3];
> +  opt_for_objcopy[0] = XALLOCAVEC (char, sizeof ("_binary__start=")
> +					 + strlen (symbol_name)

Thus you can use it here etc. (or rename i to symbol_name_len).

> +					 + strlen (symbols[0]));
> +  opt_for_objcopy[1] = XALLOCAVEC (char, sizeof ("_binary__end=")
> +					 + strlen (symbol_name)

Likewise.

> +					 + strlen (symbols[1]));
> +  opt_for_objcopy[2] = XALLOCAVEC (char, sizeof ("_binary__size=")
> +					 + strlen (symbol_name)

Likewise.

> +					 + strlen (symbols[2]));
> +  sprintf (opt_for_objcopy[0], "_binary_%s_start=%s", symbol_name, symbols[0]);
> +  sprintf (opt_for_objcopy[1], "_binary_%s_end=%s", symbol_name, symbols[1]);
> +  sprintf (opt_for_objcopy[2], "_binary_%s_size=%s", symbol_name, symbols[2]);
> +
> +  obstack_init (&argv_obstack);
> +  obstack_ptr_grow (&argv_obstack, "objcopy");
> +  obstack_ptr_grow (&argv_obstack, target_so_filename);
> +  obstack_ptr_grow (&argv_obstack, "--redefine-sym");
> +  obstack_ptr_grow (&argv_obstack, opt_for_objcopy[0]);
> +  obstack_ptr_grow (&argv_obstack, "--redefine-sym");
> +  obstack_ptr_grow (&argv_obstack, opt_for_objcopy[1]);
> +  obstack_ptr_grow (&argv_obstack, "--redefine-sym");
> +  obstack_ptr_grow (&argv_obstack, opt_for_objcopy[2]);
> +  obstack_ptr_grow (&argv_obstack, NULL);
> +  new_argv = XOBFINISH (&argv_obstack, char **);

Why do you use an obstack for an array of pointers where you know
you have exactly 9 pointers?  Wouldn't
  char *new_argv[9];
and just pointer assignments be better?

> +  fork_execute (new_argv[0], new_argv, false);
> +  obstack_free (&argv_obstack, NULL);
> +
> +  return target_so_filename;
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  progname = "mkoffload-intelmic";
> +  gcc_init_libintl ();
> +  diagnostic_initialize (global_dc, 0);
> +
> +  if (atexit (mkoffload_atexit) != 0)
> +    fatal_error ("atexit failed");
> +
> +  const char *host_compiler = getenv ("COLLECT_GCC");
> +  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);
> +
> +  /* We may be called with all the arguments stored in some file and
> +     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=", strlen ("-foffload-abi=")) == 0)
> +      {
> +	if (strstr (argv[i], "ilp32"))
> +	  target_ilp32 = true;
> +	else if (!strstr (argv[i], "lp64"))
> +	  fatal_error ("unrecognizable argument of option -foffload-abi");
> +	break;
> +      }
> +
> +  const char *target_so_filename
> +    = prepare_target_image (target_compiler, argc, argv);
> +
> +  const char *host_descr_filename = generate_host_descr_file (host_compiler);
> +
> +  /* Perform partial linking for the target image and host side descriptor.
> +     As a result we'll get a finalized object file with all offload data.  */
> +  struct obstack argv_obstack;
> +  obstack_init (&argv_obstack);
> +  obstack_ptr_grow (&argv_obstack, "ld");
> +  if (target_ilp32)
> +    {
> +      obstack_ptr_grow (&argv_obstack, "-m");
> +      obstack_ptr_grow (&argv_obstack, "elf_i386");
> +    }
> +  obstack_ptr_grow (&argv_obstack, "-r");
> +  obstack_ptr_grow (&argv_obstack, host_descr_filename);
> +  obstack_ptr_grow (&argv_obstack, target_so_filename);
> +  obstack_ptr_grow (&argv_obstack, "-o");
> +  obstack_ptr_grow (&argv_obstack, out_obj_filename);
> +  obstack_ptr_grow (&argv_obstack, NULL);
> +  char **new_argv = XOBFINISH (&argv_obstack, char **);

Similarly (well, here it is not constant, still, you know small upper bound
and can just use some int index you ++ in each assignment.

> +  /* Run objcopy on the resultant object file to localize generated symbols
> +     to avoid conflicting between different DSO and an executable.  */
> +  obstack_init (&argv_obstack);
> +  obstack_ptr_grow (&argv_obstack, "objcopy");
> +  obstack_ptr_grow (&argv_obstack, "-L");
> +  obstack_ptr_grow (&argv_obstack, symbols[0]);
> +  obstack_ptr_grow (&argv_obstack, "-L");
> +  obstack_ptr_grow (&argv_obstack, symbols[1]);
> +  obstack_ptr_grow (&argv_obstack, "-L");
> +  obstack_ptr_grow (&argv_obstack, symbols[2]);
> +  obstack_ptr_grow (&argv_obstack, out_obj_filename);
> +  obstack_ptr_grow (&argv_obstack, NULL);
> +  new_argv = XOBFINISH (&argv_obstack, char **);
> +  fork_execute (new_argv[0], new_argv, false);
> +  obstack_free (&argv_obstack, NULL);

Likewise.

	Jakub