This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH 1/4] Add mkoffload for Intel MIC

From: Bernd Schmidt <bschmidt at redhat dot com>
To: Ilya Verbin <iverbin at gmail dot com>
Cc: Thomas Schwinge <thomas at codesourcery dot com>, gcc-patches at gcc dot gnu dot org, Jakub Jelinek <jakub at redhat dot com>, Kirill Yukhin <kirill dot yukhin at gmail dot com>
Date: Mon, 28 Sep 2015 13:27:32 +0200
Subject: Re: [PATCH 1/4] Add mkoffload for Intel MIC
Authentication-results: sourceware.org; auth=none
References: <20141021171323 dot GA47586 at msticlxl57 dot ims dot intel dot com> <20141021171602 dot GB47586 at msticlxl57 dot ims dot intel dot com> <20141022082103 dot GH10376 at tucnak dot redhat dot com> <20141022185701 dot GA21398 at msticlxl57 dot ims dot intel dot com> <87r3ljuez8 dot fsf at kepler dot schwinge dot homeip dot net> <5609106F dot 9040707 at redhat dot com> <560911CF dot 2010000 at redhat dot com> <20150928112548 dot GA25312 at msticlxl57 dot ims dot intel dot com>

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] = NULL;
+  objcopy_argv[objcopy_argc++] = NULL;
+  gcc_checking_assert (objcopy_argc <= 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 same
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 review:
https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02210.html

That's unfortunate, I think that made the code less future-proof. IMO weshould revert to the obstack method especially if Thomas -v patch goes in.



Bernd

Follow-Ups:
- Re: [PATCH 1/4] Add mkoffload for Intel MIC
  - From: Jakub Jelinek
- Refactor intelmic-mkoffload.c argv building to use obstacks (was: [PATCH 1/4] Add mkoffload for Intel MIC)
  - From: Thomas Schwinge

References:
- Re: [PATCH 1/4] Add mkoffload for Intel MIC
  - From: Thomas Schwinge
- Re: [PATCH 1/4] Add mkoffload for Intel MIC
  - From: Bernd Schmidt
- Re: [PATCH 1/4] Add mkoffload for Intel MIC
  - From: Bernd Schmidt
- Re: [PATCH 1/4] Add mkoffload for Intel MIC
  - From: Ilya Verbin

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]