From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122266 invoked by alias); 30 Sep 2015 16:47:14 -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 122256 invoked by uid 89); 30 Sep 2015 16:47:13 -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; Wed, 30 Sep 2015 16:47:07 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-03.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZhKX0-0001kg-Mg from Thomas_Schwinge@mentor.com ; Wed, 30 Sep 2015 09:47:03 -0700 Received: from feldtkeller.schwinge.homeip.net (137.202.0.76) by SVR-IES-FEM-03.mgc.mentorg.com (137.202.0.108) with Microsoft SMTP Server id 14.3.224.2; Wed, 30 Sep 2015 17:47:01 +0100 From: Thomas Schwinge To: , Bernd Schmidt , Ilya Verbin CC: Jakub Jelinek , Kirill Yukhin Subject: Refactor intelmic-mkoffload.c argv building to use obstacks (was: [PATCH 1/4] Add mkoffload for Intel MIC) In-Reply-To: <56092424.6000808@redhat.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> <87r3ljuez8.fsf@kepler.schwinge.homeip.net> <5609106F.9040707@redhat.com> <560911CF.2010000@redhat.com> <20150928112548.GA25312@msticlxl57.ims.intel.com> <56092424.6000808@redhat.com> User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/24.5.1 (i586-pc-linux-gnu) Date: Wed, 30 Sep 2015 17:55:00 -0000 Message-ID: <87h9mbsvlt.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/msg02374.txt.bz2 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-length: 10315 Hi! On Mon, 28 Sep 2015 13:27:32 +0200, Bernd Schmidt wro= te: > On 09/28/2015 01:25 PM, Ilya Verbin wrote: > > On Mon, Sep 28, 2015 at 12:09:19 +0200, Bernd Schmidt wrote: > >> On 09/28/2015 12:03 PM, Bernd Schmidt wrote: > >>> On 09/28/2015 10:26 AM, Thomas Schwinge wrote: > >>>> - objcopy_argv[8] =3D NULL; > >>>> + objcopy_argv[objcopy_argc++] =3D NULL; > >>>> + gcc_checking_assert (objcopy_argc <=3D OBJCOPY_ARGC_MAX); > >>> > >>> On its own this is not an improvement - you're trading a compile time > >>> error for a runtime error. So, what is the other change this is > >>> preparing for? > >> > >> Ok, I now see the other patch. But I also see that other code in the s= ame > >> file and in the nvptx mkoffload is using the obstack_ptr_grow method to > >> build argv arrays, I think that would be preferrable to this. > > > > I've removed obstack_ptr_grow for arrays with known sizes after this re= view: > > https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02210.html >=20 > That's unfortunate, I think that made the code less future-proof. IMO we= =20 > should revert to the obstack method especially if Thomas -v patch goes in. Given that the discussion has settled in favor of using obstacks, I have committed the following in r228300: commit 99043644772bdc4b76d44058014d664ce27867f7 Author: tschwinge Date: Wed Sep 30 16:42:22 2015 +0000 Refactor intelmic-mkoffload.c argv building to use obstacks =20=20=20=20 That is, restore and adapt the code as originally proposed. =20=20=20=20 gcc/ * config/i386/intelmic-mkoffload.c (generate_host_descr_file) (prepare_target_image, main): Refactor argv building to use obstacks. =20=20=20=20 git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@228300 138bc75d-0d04-04= 10-961f-82ee72b054a4 --- gcc/ChangeLog | 8 +++ gcc/config/i386/intelmic-mkoffload.c | 132 +++++++++++++++++++------------= ---- 2 files changed, 80 insertions(+), 60 deletions(-) diff --git gcc/ChangeLog gcc/ChangeLog index 86f81d6..15f84ab 100644 --- gcc/ChangeLog +++ gcc/ChangeLog @@ -1,3 +1,11 @@ +2015-09-30 Thomas Schwinge + Ilya Verbin + Andrey Turetskiy + + * config/i386/intelmic-mkoffload.c (generate_host_descr_file) + (prepare_target_image, main): Refactor argv building to use + obstacks. + 2015-09-30 Ulrich Weigand =20 * config/spu/spu-protos.h (spu_expand_atomic_op): Add prototype. diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mk= offload.c index 065d408..ae88ecd 100644 --- gcc/config/i386/intelmic-mkoffload.c +++ gcc/config/i386/intelmic-mkoffload.c @@ -379,29 +379,31 @@ generate_host_descr_file (const char *host_compiler) =20 fclose (src_file); =20 - unsigned new_argc =3D 0; - const char *new_argv[9]; - new_argv[new_argc++] =3D host_compiler; - new_argv[new_argc++] =3D "-c"; - new_argv[new_argc++] =3D "-fPIC"; - new_argv[new_argc++] =3D "-shared"; + struct obstack argv_obstack; + obstack_init (&argv_obstack); + obstack_ptr_grow (&argv_obstack, host_compiler); + obstack_ptr_grow (&argv_obstack, "-c"); + obstack_ptr_grow (&argv_obstack, "-fPIC"); + obstack_ptr_grow (&argv_obstack, "-shared"); switch (offload_abi) { case OFFLOAD_ABI_LP64: - new_argv[new_argc++] =3D "-m64"; + obstack_ptr_grow (&argv_obstack, "-m64"); break; case OFFLOAD_ABI_ILP32: - new_argv[new_argc++] =3D "-m32"; + obstack_ptr_grow (&argv_obstack, "-m32"); break; default: gcc_unreachable (); } - new_argv[new_argc++] =3D src_filename; - new_argv[new_argc++] =3D "-o"; - new_argv[new_argc++] =3D obj_filename; - new_argv[new_argc++] =3D NULL; + obstack_ptr_grow (&argv_obstack, src_filename); + obstack_ptr_grow (&argv_obstack, "-o"); + obstack_ptr_grow (&argv_obstack, obj_filename); + obstack_ptr_grow (&argv_obstack, NULL); =20 - fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); + char **argv =3D XOBFINISH (&argv_obstack, char **); + fork_execute (argv[0], argv, false); + obstack_free (&argv_obstack, NULL); =20 return obj_filename; } @@ -448,29 +450,31 @@ prepare_target_image (const char *target_compiler, in= t argc, char **argv) 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"; + obstack_init (&argv_obstack); + obstack_ptr_grow (&argv_obstack, "objcopy"); + obstack_ptr_grow (&argv_obstack, "-B"); + obstack_ptr_grow (&argv_obstack, "i386"); + obstack_ptr_grow (&argv_obstack, "-I"); + obstack_ptr_grow (&argv_obstack, "binary"); + obstack_ptr_grow (&argv_obstack, "-O"); switch (offload_abi) { case OFFLOAD_ABI_LP64: - objcopy_argv[6] =3D "elf64-x86-64"; + obstack_ptr_grow (&argv_obstack, "elf64-x86-64"); break; case OFFLOAD_ABI_ILP32: - objcopy_argv[6] =3D "elf32-i386"; + obstack_ptr_grow (&argv_obstack, "elf32-i386"); break; default: gcc_unreachable (); } - 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; - fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false= ); + obstack_ptr_grow (&argv_obstack, target_so_filename); + obstack_ptr_grow (&argv_obstack, "--rename-section"); + obstack_ptr_grow (&argv_obstack, rename_section_opt); + obstack_ptr_grow (&argv_obstack, NULL); + char **new_argv =3D XOBFINISH (&argv_obstack, char **); + fork_execute (new_argv[0], new_argv, false); + obstack_free (&argv_obstack, NULL); =20 /* Objcopy has created symbols, containing the input file name with non-alphanumeric characters replaced by underscores. @@ -500,16 +504,19 @@ 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; - fork_execute (objcopy_argv[0], CONST_CAST (char **, objcopy_argv), false= ); + 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 **); + fork_execute (new_argv[0], new_argv, false); + obstack_free (&argv_obstack, NULL); =20 return target_so_filename; } @@ -562,41 +569,46 @@ main (int argc, char **argv) =20 /* 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_argv[new_argc++] =3D "ld"; - new_argv[new_argc++] =3D "-m"; + struct obstack argv_obstack; + obstack_init (&argv_obstack); + obstack_ptr_grow (&argv_obstack, "ld"); + obstack_ptr_grow (&argv_obstack, "-m"); switch (offload_abi) { case OFFLOAD_ABI_LP64: - new_argv[new_argc++] =3D "elf_x86_64"; + obstack_ptr_grow (&argv_obstack, "elf_x86_64"); break; case OFFLOAD_ABI_ILP32: - new_argv[new_argc++] =3D "elf_i386"; + obstack_ptr_grow (&argv_obstack, "elf_i386"); break; default: gcc_unreachable (); } - new_argv[new_argc++] =3D "--relocatable"; - new_argv[new_argc++] =3D host_descr_filename; - new_argv[new_argc++] =3D target_so_filename; - new_argv[new_argc++] =3D "-o"; - new_argv[new_argc++] =3D out_obj_filename; - new_argv[new_argc++] =3D NULL; - fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); + obstack_ptr_grow (&argv_obstack, "--relocatable"); + 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 **); + fork_execute (new_argv[0], new_argv, false); + obstack_free (&argv_obstack, NULL); =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; - fork_execute (new_argv[0], CONST_CAST (char **, new_argv), false); + 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 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 iQEcBAEBAgAGBQJWDBH+AAoJEK3/DN1sMFFtwlwH/ir9C0YGDDH/pw6N1lncGIAq x5OyQKR4pqB4LRsz3MhqYntKs5POU3U+i2xIJsc3f1IIPRi0E1FjMsBNBR1HNXt/ flXfWApRT3+cyVkriEVY88jThkmBt5lGsv2vySPlY8B2AqCb6r7AWAtO5LZVcinB SAcDG4Xspz18yD7I9RJkFvO2xgDNwIAFW6U73sbOYNQrNJ/ZI9RBOfCtCdQyjim5 8MqWsh0fmZGoCFNJks3F7pqJ4ceoXlXKeqimrsrpCptfF9FV9Vdpq2nXeobAZzSb 6Bf/ZAm/xN94v9OtLApueB964UMZ3ysu790xS3CZDYUZHFYjdN1/nPlQsCu/jpY= =9jQw -----END PGP SIGNATURE----- --=-=-=--