From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Archive: List-Post: List-Help: Sender: gcc-patches-owner@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 09:39:00 -0000 Message-ID: <87r3ljuez8.fsf@kepler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" X-SW-Source: 2015-09/txt/msg02087.txt.bz2 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-length: 9629 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 =3D XOBFINISH (&argv_obstack, char **); > >=20 > > 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? >=20 > Yes, done. >=20 > > > + /* Perform partial linking for the target image and host side desc= riptor. > > > + 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 =3D XOBFINISH (&argv_obstack, char **); > >=20 > > Similarly (well, here it is not constant, still, you know small upper b= ound > > and can just use some int index you ++ in each assignment. >=20 > Done. >=20 > > > + /* 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 =3D XOBFINISH (&argv_obstack, char **); > > > + fork_execute (new_argv[0], new_argv, false); > > > + obstack_free (&argv_obstack, NULL); > >=20 > > Likewise. >=20 > 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 =20=20=20=20 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(-) diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mk= offload.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); =20 unsigned new_argc =3D 0; - const char *new_argv[9]; +#define NEW_ARGC_MAX 9 + const char *new_argv[NEW_ARGC_MAX]; new_argv[new_argc++] =3D host_compiler; new_argv[new_argc++] =3D "-c"; new_argv[new_argc++] =3D "-fPIC"; @@ -400,6 +401,8 @@ generate_host_descr_file (const char *host_compiler) new_argv[new_argc++] =3D "-o"; new_argv[new_argc++] =3D obj_filename; new_argv[new_argc++] =3D NULL; + gcc_checking_assert (new_argc <=3D NEW_ARGC_MAX); +#undef NEW_ARGC_MAX =20 fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); =20 @@ -444,32 +447,37 @@ prepare_target_image (const char *target_compiler, in= t argc, char **argv) obstack_ptr_grow (&argv_obstack, target_so_filename); compile_for_target (&argv_obstack); =20 + unsigned objcopy_argc; +#define OBJCOPY_ARGC_MAX 11 + const char *objcopy_argv[OBJCOPY_ARGC_MAX]; + /* Run objcopy. */ char *rename_section_opt =3D XALLOCAVEC (char, sizeof (".data=3D") + strlen (image_section_name= )); sprintf (rename_section_opt, ".data=3D%s", image_section_name); - const char *objcopy_argv[11]; - objcopy_argv[0] =3D "objcopy"; - objcopy_argv[1] =3D "-B"; - objcopy_argv[2] =3D "i386"; - objcopy_argv[3] =3D "-I"; - objcopy_argv[4] =3D "binary"; - objcopy_argv[5] =3D "-O"; + objcopy_argc =3D 0; + objcopy_argv[objcopy_argc++] =3D "objcopy"; + objcopy_argv[objcopy_argc++] =3D "-B"; + objcopy_argv[objcopy_argc++] =3D "i386"; + objcopy_argv[objcopy_argc++] =3D "-I"; + objcopy_argv[objcopy_argc++] =3D "binary"; + objcopy_argv[objcopy_argc++] =3D "-O"; switch (offload_abi) { case OFFLOAD_ABI_LP64: - objcopy_argv[6] =3D "elf64-x86-64"; + objcopy_argv[objcopy_argc++] =3D "elf64-x86-64"; break; case OFFLOAD_ABI_ILP32: - objcopy_argv[6] =3D "elf32-i386"; + objcopy_argv[objcopy_argc++] =3D "elf32-i386"; break; default: abort (); } - objcopy_argv[7] =3D target_so_filename; - objcopy_argv[8] =3D "--rename-section"; - objcopy_argv[9] =3D rename_section_opt; - objcopy_argv[10] =3D NULL; + objcopy_argv[objcopy_argc++] =3D target_so_filename; + objcopy_argv[objcopy_argc++] =3D "--rename-section"; + objcopy_argv[objcopy_argc++] =3D rename_section_opt; + objcopy_argv[objcopy_argc++] =3D NULL; + gcc_checking_assert (objcopy_argc <=3D OBJCOPY_ARGC_MAX); fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false= ); =20 /* Objcopy has created symbols, containing the input file name with @@ -500,17 +508,21 @@ prepare_target_image (const char *target_compiler, in= t argc, char **argv) sprintf (opt_for_objcopy[1], "_binary_%s_end=3D%s", symbol_name, symbols= [1]); sprintf (opt_for_objcopy[2], "_binary_%s_size=3D%s", symbol_name, symbol= s[2]); =20 - objcopy_argv[0] =3D "objcopy"; - objcopy_argv[1] =3D target_so_filename; - objcopy_argv[2] =3D "--redefine-sym"; - objcopy_argv[3] =3D opt_for_objcopy[0]; - objcopy_argv[4] =3D "--redefine-sym"; - objcopy_argv[5] =3D opt_for_objcopy[1]; - objcopy_argv[6] =3D "--redefine-sym"; - objcopy_argv[7] =3D opt_for_objcopy[2]; - objcopy_argv[8] =3D NULL; + objcopy_argc =3D 0; + objcopy_argv[objcopy_argc++] =3D "objcopy"; + objcopy_argv[objcopy_argc++] =3D target_so_filename; + objcopy_argv[objcopy_argc++] =3D "--redefine-sym"; + objcopy_argv[objcopy_argc++] =3D opt_for_objcopy[0]; + objcopy_argv[objcopy_argc++] =3D "--redefine-sym"; + objcopy_argv[objcopy_argc++] =3D opt_for_objcopy[1]; + objcopy_argv[objcopy_argc++] =3D "--redefine-sym"; + objcopy_argv[objcopy_argc++] =3D opt_for_objcopy[2]; + objcopy_argv[objcopy_argc++] =3D NULL; + gcc_checking_assert (objcopy_argc <=3D OBJCOPY_ARGC_MAX); fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false= ); =20 +#undef OBJCOPY_ARGC_MAX + return target_so_filename; } =20 @@ -560,10 +572,13 @@ main (int argc, char **argv) =20 const char *host_descr_filename =3D generate_host_descr_file (host_compi= ler); =20 + 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 =3D 0; - const char *new_argv[9]; + new_argc =3D 0; new_argv[new_argc++] =3D "ld"; new_argv[new_argc++] =3D "-m"; switch (offload_abi) @@ -583,20 +598,25 @@ main (int argc, char **argv) new_argv[new_argc++] =3D "-o"; new_argv[new_argc++] =3D out_obj_filename; new_argv[new_argc++] =3D NULL; + gcc_checking_assert (new_argc <=3D NEW_ARGC_MAX); fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); =20 /* Run objcopy on the resultant object file to localize generated symbols to avoid conflicting between different DSO and an executable. */ - new_argv[0] =3D "objcopy"; - new_argv[1] =3D "-L"; - new_argv[2] =3D symbols[0]; - new_argv[3] =3D "-L"; - new_argv[4] =3D symbols[1]; - new_argv[5] =3D "-L"; - new_argv[6] =3D symbols[2]; - new_argv[7] =3D out_obj_filename; - new_argv[8] =3D NULL; + new_argc =3D 0; + new_argv[new_argc++] =3D "objcopy"; + new_argv[new_argc++] =3D "-L"; + new_argv[new_argc++] =3D symbols[0]; + new_argv[new_argc++] =3D "-L"; + new_argv[new_argc++] =3D symbols[1]; + new_argv[new_argc++] =3D "-L"; + new_argv[new_argc++] =3D symbols[2]; + new_argv[new_argc++] =3D out_obj_filename; + new_argv[new_argc++] =3D NULL; + gcc_checking_assert (new_argc <=3D NEW_ARGC_MAX); fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); =20 +#undef NEW_ARGC_MAX + return 0; } Gr=C3=BC=C3=9Fe, Thomas --=-=-= Content-Type: application/pgp-signature; name="signature.asc" Content-length: 472 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWCPmrAAoJEK3/DN1sMFFtl8oIAJtUCwbv3xCLG1uy9isxliu7 ptVX7I3CyLQXAP/9QcElLWTPYVtfwFAIdynJgvo8fJ8jJ8OMUmxfKcvmSxWG6GDf ntR3uUxm06L3ovzscAukEHbHVdAfBHxNqezR4JroX56AuKGiVfseJ0TNvaDznV1c NobP6mYAJGF5PM1VnouAzwUKAnsGg/VNk4zjHBJHIoQt6OUEenqnP/tbyfMR0tlr OTfnDnDtwn+m7zVYZ41JZHy/VWxETBYJ+Sg5Wv1grmSjs+RFGxYGAmq7IzggnDsq 6O42mfH6iwHv6ZPdKiOZ3DR5L626BLMpHZ1+h1uU9vvMH9psM31zzGxSeTiwlH0= =hox2 -----END PGP SIGNATURE----- --=-=-=--