From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18174 invoked by alias); 22 Oct 2014 08:21:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 18162 invoked by uid 89); 22 Oct 2014 08:21:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 22 Oct 2014 08:21:10 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9M8L95l024682 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 22 Oct 2014 04:21:09 -0400 Received: from tucnak.zalov.cz (ovpn-116-116.ams2.redhat.com [10.36.116.116]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9M8L7xI017344 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Wed, 22 Oct 2014 04:21:08 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.9/8.14.9) with ESMTP id s9M8L5h5014560; Wed, 22 Oct 2014 10:21:06 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.9/8.14.9/Submit) id s9M8L3kc014559; Wed, 22 Oct 2014 10:21:03 +0200 Date: Wed, 22 Oct 2014 08:27:00 -0000 From: Jakub Jelinek To: Ilya Verbin Cc: gcc-patches@gcc.gnu.org, Kirill Yukhin , Andrey Turetskiy Subject: Re: [PATCH 1/4] Add mkoffload for Intel MIC Message-ID: <20141022082103.GH10376@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20141021171323.GA47586@msticlxl57.ims.intel.com> <20141021171602.GB47586@msticlxl57.ims.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141021171602.GB47586@msticlxl57.ims.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg02211.txt.bz2 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