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: gcc corrections for better pie support


On Mon, 8 Nov 2004, Zack Weinberg wrote:

> I said "submit each logically distinct change separately".  That means
> sending each patch in a separate email.  Furthermore, you broke up the
> patch along structural lines, not logical lines.  This is less
> convenient to review.  You listed six problems with the existing code;
> ideally you would send six independent patches, each of which
> completely solves exactly one of those problems.

if gcc would have the same config for all archs (at least "looking" the 
same), it would be no problem, the problem lies in the different 
approaches used to create the specs for different archs (you can compare 
it for arm and sparc for ex.)
Well, I don't want to sync that stuff up, because I am unable to do 
that ;-(

> Also, it is silly to send small patches like this in a .tar.bz2.
> Please use uncompressed text/plain attachments, or (if you are certain
> that your email client will not mangle whitespace) include the patch
> in the body of the message.  Please also read the patch-submission
> guidelines at <http://gcc.gnu.org/contribute.html> and
> <http://gcc.gnu.org/codingconventions.html>, with particular attention
> to the ChangeLog requirement.

I will..

> As a suggestion, I see that you moved definitions of __pic__ and/or
> __PIC__ from specs strings to TARGET_CPP_OS_BUILTINS.  This is the
> right thing to do, but it is not the perfect thing.  Those definitions
> are, or should be, consistent across all systems, so they should be
> handled by c-cppbuiltin.c in a generic manner.  Please consider making
> that change instead.

I have no C knowledge, so don't ask me such thing, everywhere where I have 
done it, I have used the approach used for that specific arch (I have 
really looked hard where to put it), and as I said, almost all archs 
differ and put the definitions at different locations and also put them 
into different vars.

You could check to see w/ `grep "f PIC" in gcc/config/*/*` how many times 
this shows up currently and in what context, than you'll see what I mean 
("K PIC" is also good ;)

> 
> >> reports in GCC Bugzilla (http://gcc.gnu.org/bugzilla).  File
> >> separate bug reports for each logically distinct bug.
> >
> > Well, first I will wait to see what the reactions to these patches
> > are, if I see any activity/comments, then I will do that, else it is
> > useless.
> 
> Our process does not work that way.  You substantially increase the
> odds of your patches being reviewed if you file bug reports now.

I'll do that...

Thanks, Peter

> I also recommend that you bring your patches to the attention of Jakub
> Jelinek <jakub@redhat.com>, as he is the original contributor of PIE
> support.  He is probably the only person with sufficient context to
> determine whether your spec changes are correct.

-- 
Peter S. Mazinger <ps dot m at gmx dot net>           ID: 0xA5F059F2
Key fingerprint = 92A4 31E1 56BC 3D5A 2D08  BB6E C389 975E A5F0 59F2


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