From patchwork Mon Sep 28 08:26:19 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 523261 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id CEEB11401AF for ; Mon, 28 Sep 2015 18:26:42 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Kn03tZU1; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; q=dns; s=default; b=gYK1k2re4RJ98wsZ lQrEgAC7XdzM2nuNgmRWKEYJx7jiQj2RualpQIP7R9i3Iy8ZrnSoRoCYvYYCxvmy 1gEGOj/tFI7u6BU+zvrJP9RBNSO8EpZp151YxY9ShFEzIs7VrNPrliizkJgGjTpA vTB6rZohucNgRBw2KwueJRrAUjQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=default; bh=B+0I55fKMOC6Wy6HET69YY 4zduc=; b=Kn03tZU1p1ykc3zTiCRUWbRB/UQ0UaAVf2RAieA7r6yeoKu9zvhvvt hzdPoOw7ysW8C+tk9iE7lrzELf7SUtE3/vTdgcEuSDcS1jXX+5fm+zMBdqDw9BEC YLB0spLi8pkcT/MX7++nduuxdIlRaMkx4aiuWD6LpIWE9qc3oGdmI= Received: (qmail 90424 invoked by alias); 28 Sep 2015 08:26:35 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 90409 invoked by uid 89); 28 Sep 2015 08:26:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 28 Sep 2015 08:26:32 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZgTlU-00035i-Eo from Thomas_Schwinge@mentor.com ; Mon, 28 Sep 2015 01:26:28 -0700 Received: from feldtkeller.schwinge.homeip.net (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Mon, 28 Sep 2015 09:26:27 +0100 From: Thomas Schwinge To: , Ilya Verbin , Jakub Jelinek CC: Kirill Yukhin , Andrey Turetskiy , Bernd Schmidt Subject: Re: [PATCH 1/4] Add mkoffload for Intel MIC In-Reply-To: <20141022185701.GA21398@msticlxl57.ims.intel.com> References: <20141021171323.GA47586@msticlxl57.ims.intel.com> <20141021171602.GB47586@msticlxl57.ims.intel.com> <20141022082103.GH10376@tucnak.redhat.com> <20141022185701.GA21398@msticlxl57.ims.intel.com> User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/24.5.1 (i586-pc-linux-gnu) Date: Mon, 28 Sep 2015 10:26:19 +0200 Message-ID: <87r3ljuez8.fsf@kepler.schwinge.homeip.net> MIME-Version: 1.0 Hi! On Wed, 22 Oct 2014 22:57:01 +0400, Ilya Verbin wrote: > On 22 Oct 09:57, Jakub Jelinek wrote: > > On Wed, Oct 22, 2014 at 02:30:44AM +0400, Ilya Verbin wrote: > > > + 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? > > Yes, done. > > > > + /* 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. > > Done. > > > > + /* 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. > > Done. After approval for "Use gcc/coretypes.h:enum offload_abi in mkoffloads", , I'd like to commit the following refactoring patch to trunk, in preparation for another change: commit 91fbe15ce2e539a4017f65cc167b362a4b4e4553 Author: Thomas Schwinge Date: Tue Aug 4 14:06:39 2015 +0200 Refactor intelmic-mkoffload.c argv building gcc/ * config/i386/intelmic-mkoffload.c (generate_host_descr_file) (prepare_target_image, main): Refactor argv building. --- gcc/config/i386/intelmic-mkoffload.c | 88 +++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 34 deletions(-) Grüße, Thomas diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mkoffload.c index 8028584..8d5af0d 100644 --- gcc/config/i386/intelmic-mkoffload.c +++ gcc/config/i386/intelmic-mkoffload.c @@ -380,7 +380,8 @@ generate_host_descr_file (const char *host_compiler) fclose (src_file); unsigned new_argc = 0; - const char *new_argv[9]; +#define NEW_ARGC_MAX 9 + const char *new_argv[NEW_ARGC_MAX]; new_argv[new_argc++] = host_compiler; new_argv[new_argc++] = "-c"; new_argv[new_argc++] = "-fPIC"; @@ -400,6 +401,8 @@ generate_host_descr_file (const char *host_compiler) new_argv[new_argc++] = "-o"; new_argv[new_argc++] = obj_filename; new_argv[new_argc++] = NULL; + gcc_checking_assert (new_argc <= NEW_ARGC_MAX); +#undef NEW_ARGC_MAX fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); @@ -444,32 +447,37 @@ prepare_target_image (const char *target_compiler, int argc, char **argv) obstack_ptr_grow (&argv_obstack, target_so_filename); compile_for_target (&argv_obstack); + unsigned objcopy_argc; +#define OBJCOPY_ARGC_MAX 11 + const char *objcopy_argv[OBJCOPY_ARGC_MAX]; + /* Run objcopy. */ char *rename_section_opt = XALLOCAVEC (char, sizeof (".data=") + strlen (image_section_name)); sprintf (rename_section_opt, ".data=%s", image_section_name); - const char *objcopy_argv[11]; - objcopy_argv[0] = "objcopy"; - objcopy_argv[1] = "-B"; - objcopy_argv[2] = "i386"; - objcopy_argv[3] = "-I"; - objcopy_argv[4] = "binary"; - objcopy_argv[5] = "-O"; + objcopy_argc = 0; + objcopy_argv[objcopy_argc++] = "objcopy"; + objcopy_argv[objcopy_argc++] = "-B"; + objcopy_argv[objcopy_argc++] = "i386"; + objcopy_argv[objcopy_argc++] = "-I"; + objcopy_argv[objcopy_argc++] = "binary"; + objcopy_argv[objcopy_argc++] = "-O"; switch (offload_abi) { case OFFLOAD_ABI_LP64: - objcopy_argv[6] = "elf64-x86-64"; + objcopy_argv[objcopy_argc++] = "elf64-x86-64"; break; case OFFLOAD_ABI_ILP32: - objcopy_argv[6] = "elf32-i386"; + objcopy_argv[objcopy_argc++] = "elf32-i386"; break; default: abort (); } - objcopy_argv[7] = target_so_filename; - objcopy_argv[8] = "--rename-section"; - objcopy_argv[9] = rename_section_opt; - objcopy_argv[10] = NULL; + objcopy_argv[objcopy_argc++] = target_so_filename; + objcopy_argv[objcopy_argc++] = "--rename-section"; + objcopy_argv[objcopy_argc++] = rename_section_opt; + objcopy_argv[objcopy_argc++] = NULL; + gcc_checking_assert (objcopy_argc <= OBJCOPY_ARGC_MAX); fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false); /* Objcopy has created symbols, containing the input file name with @@ -500,17 +508,21 @@ prepare_target_image (const char *target_compiler, int argc, char **argv) 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]); - objcopy_argv[0] = "objcopy"; - objcopy_argv[1] = target_so_filename; - objcopy_argv[2] = "--redefine-sym"; - objcopy_argv[3] = opt_for_objcopy[0]; - objcopy_argv[4] = "--redefine-sym"; - objcopy_argv[5] = opt_for_objcopy[1]; - objcopy_argv[6] = "--redefine-sym"; - objcopy_argv[7] = opt_for_objcopy[2]; - objcopy_argv[8] = NULL; + objcopy_argc = 0; + objcopy_argv[objcopy_argc++] = "objcopy"; + objcopy_argv[objcopy_argc++] = target_so_filename; + objcopy_argv[objcopy_argc++] = "--redefine-sym"; + objcopy_argv[objcopy_argc++] = opt_for_objcopy[0]; + objcopy_argv[objcopy_argc++] = "--redefine-sym"; + objcopy_argv[objcopy_argc++] = opt_for_objcopy[1]; + objcopy_argv[objcopy_argc++] = "--redefine-sym"; + objcopy_argv[objcopy_argc++] = opt_for_objcopy[2]; + objcopy_argv[objcopy_argc++] = NULL; + gcc_checking_assert (objcopy_argc <= OBJCOPY_ARGC_MAX); fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false); +#undef OBJCOPY_ARGC_MAX + return target_so_filename; } @@ -560,10 +572,13 @@ main (int argc, char **argv) const char *host_descr_filename = generate_host_descr_file (host_compiler); + unsigned new_argc; +#define NEW_ARGC_MAX 9 + const char *new_argv[NEW_ARGC_MAX]; + /* 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. */ - unsigned new_argc = 0; - const char *new_argv[9]; + new_argc = 0; new_argv[new_argc++] = "ld"; new_argv[new_argc++] = "-m"; switch (offload_abi) @@ -583,20 +598,25 @@ main (int argc, char **argv) new_argv[new_argc++] = "-o"; new_argv[new_argc++] = out_obj_filename; new_argv[new_argc++] = NULL; + gcc_checking_assert (new_argc <= NEW_ARGC_MAX); fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); /* Run objcopy on the resultant object file to localize generated symbols to avoid conflicting between different DSO and an executable. */ - new_argv[0] = "objcopy"; - new_argv[1] = "-L"; - new_argv[2] = symbols[0]; - new_argv[3] = "-L"; - new_argv[4] = symbols[1]; - new_argv[5] = "-L"; - new_argv[6] = symbols[2]; - new_argv[7] = out_obj_filename; - new_argv[8] = NULL; + new_argc = 0; + new_argv[new_argc++] = "objcopy"; + new_argv[new_argc++] = "-L"; + new_argv[new_argc++] = symbols[0]; + new_argv[new_argc++] = "-L"; + new_argv[new_argc++] = symbols[1]; + new_argv[new_argc++] = "-L"; + new_argv[new_argc++] = symbols[2]; + new_argv[new_argc++] = out_obj_filename; + new_argv[new_argc++] = NULL; + gcc_checking_assert (new_argc <= NEW_ARGC_MAX); fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); +#undef NEW_ARGC_MAX + return 0; }