Re: darwin cross compiling breakage

[Geoffrey Keating <geoffk at apple dot com>]

On 11/06/2006, at 1:33 PM, Mark Mitchell wrote:

> Geoffrey Keating wrote:
> > On 09/06/2006, at 5:25 PM, Steven Bosscher wrote:
> >
> > > On Saturday 10 June 2006 02:03, Geoffrey Keating wrote:
> > > > Mike Stump <mrs@apple.com> writes:
> > > > > On Jun 6, 2006, at 5:57 AM, Mike Stump wrote:
> > > > > > The recent changes to support code generation for the current
> > > > > > machine on x86 has broken darwin's ppc x i386 compiler.
> > > > >
> > > > > Ping?
> > > > >
> > > > > http://gcc.gnu.org/ml/gcc-patches/2006-06/msg00292.html
> > > >
> > > > This is OK, except lose the APPLE LOCAL markers.
> > >
> > > How can this be OK when people raise issues with the patch?
> >
> > "OK" does not mean that we have perfect consensus and everyone  
> > agrees.
> > It means that on balance it is better to have this patch than not.
>
> I think it's inappropriate for you to overrule Ian without trying to
> build consensus, given that this is a change to the IA32 back end.   
> You
> have GWP, and Ian does not, so it is true that you can change i386.h
> without approval, and that Ian cannot; in that sense, you outrank Ian.

One reason why I said that `"OK" does not mean that we have perfect  
consensus and everyone agrees.' is that in practise it is impossible  
for a patch reviewer to determine when such a consensus has been  
attained.  For example, people may have sent mail which has not yet  
reached or been read by the reviewer.

One should not, therefore, consider an "OK" to overrule anyone not  
explicitly mentioned in the message.  That goes double for cases like  
this one where two reviewers have sent mail about the same patch less  
than 3 minutes apart.

It would also be a good idea in future, should there appear to be a  
conflict between two reviews, to send the reviewers links to or  
copies of each other's mail.  In this fashion, much confusion may be  
avoided; for instance, in previous drafts of this message, I was  
going to ask if Stephen's nickname was "Ian" since I was not aware  
that any other "Ian" was involved in this discussion.  (I have added  
him to the CC list.)  I think we could have gone like that for maybe  
three or four rounds of e-mail before I realised that something  
wasn't making sense.

I would also comment that I don't believe it's a patch reviewer's  
responsibility to build consensus, and in fact to attempt to do so  
could create a conflict with the responsibility to give the patch a  
fair review on its merits.  An exception to this would be when a  
patch reviewer is not actually reviewing the patch, but attempting to  
reconcile other reviews in an attempt to make forward progress.

> But, I think Ian's comment:
>
> > Perhaps an intermediate approach would be to introduce a new macro,
> > like X86_SUPPORT_LOCAL, and then define that if CROSS_COMPILE is not
> > defined and __i386__ is defined, with a clear comment.
>
> suggests a path forward which is no harder than the one posted, but
> considerably clearer, as well as avoiding code duplication.

If we're talking about the best thing to do, I believe we should  
replace the condition with

#ifdef __i386__

since in the cases I've examined where this differs from #ifdef  
CROSS_COMPILE, it is more useful.  For example, when using a compiler  
hosted on i386-linux2.4 but targetting i386-linux2.2 (or so),  
CROSS_COMPILE is defined, but it is quite reasonable to want - 
mtune=current.

As for Ian's comments about CROSS_COMPILE, I would be happy to  
entertain a new and more descriptive name for this macro.  Perhaps  
CROSS_DIRECTORY_LAYOUT would be better.

Attachment: smime.p7s
Description: S/MIME cryptographic signature