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
