RE: [PATCH 5/7]: Ping2: Merge from Stack Branch - other ports

["Lu, Hongjiu" <hongjiu dot lu at intel dot com>]

Hi Ian,

This patch has been made obsolete by

http://gcc.gnu.org/ml/gcc-cvs/2008-05/msg01089.html

It is no longer needed.

Thanks.


H.J.
hongjiu.lu@intel.com

>-----Original Message-----
>From: Ian Lance Taylor [mailto:iant@google.com]
>Sent: Friday, May 30, 2008 7:45 AM
>To: Ye, Joey
>Cc: gcc-patches@gcc.gnu.org; Lu, Hongjiu; Guo, Xuepeng
>Subject: Re: [PATCH 5/7]: Ping2: Merge from Stack Branch - other ports
>
>"Ye, Joey" <joey.ye@intel.com> writes:
>
>> OK to mainline?
>>
>> 2008-05-19  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         * config/bfin/bfin.c (bfin_local_alignment): Use new
>> 	LOCAL_ALIGNMENT macro.
>>         * config/bfin/bfin.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/bfin/bfin-protos.h (bfin_local_alignment): Likewise.
>>         * config/i386/i386.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/i386/i386-protos.h (ix86_local_alignment): Likewise.
>>         * config/mips/mips.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/mmix/mmix.c (mmix_local_alignment): Likewise.
>>         * config/mmix/mmix.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/mmix/mmix-protos.h (mmix_local_alignment): Likewise.
>>         * config/rs6000/rs6000.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/score/score.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/sh/sh.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/sparc/sparc.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/spu/spu.h (LOCAL_ALIGNMENT): Likewise.
>>         * config/rs6000/rs6000.h (LOCAL_ALIGNMENT_1): New.
>>         * config/sh/sh.h (LOCAL_ALIGNMENT_1): Likewise.
>
>I don't see any big problems with this patch, but I don't see how I
>can approve it as is.  This patch changes the definition of
>LOCAL_ALIGNMENT in a number of places, but it doesn't change the uses.
>You've changed the uses in a different patch.  And you've changed the
>documentation in a third patch.
>
>Since every piece of this set of patches is being tweaked through a
>series of review steps, it's hard to keep track of the current status.
>I think that you need to present this patch as a unit which can be
>checked in all by itself.  That means providing the change to the uses
>of LOCAL_ALIGNMENT, the change to the definitions of LOCAL_ALIGNMENT,
>and the change to the documentation of LOCAL_ALIGNMENT, all in a
>single patch.
>
>Thanks, and sorry for the extra work.
>
>
>
>> Index: config/spu/spu.h
>> ===================================================================
>> --- config/spu/spu.h	(revision 135492)
>> +++ config/spu/spu.h	(working copy)
>> @@ -100,7 +100,8 @@ extern GTY(()) int spu_tune;
>>     unaligned.)  */
>>  #define DATA_ALIGNMENT(TYPE,ALIGN) ((ALIGN) > 128 ? (ALIGN) : 128)
>>  #define CONSTANT_ALIGNMENT(TYPE,ALIGN) ((ALIGN) > 128 ? (ALIGN) :
128)
>> -#define LOCAL_ALIGNMENT(TYPE,ALIGN) ((ALIGN) > 128 ? (ALIGN) : 128)
>> +#define LOCAL_ALIGNMENT(TYPE,MODE,ALIGN) \
>> +  ((!(TYPE) || (ALIGN) > 128) ? (ALIGN) : 128)
>
>Why are you testing TYPE here?
>
>
>> Index: config/mmix/mmix.c
>> ===================================================================
>> --- config/mmix/mmix.c	(revision 135492)
>> +++ config/mmix/mmix.c	(working copy)
>> @@ -271,10 +271,12 @@ mmix_constant_alignment (tree constant A
>>
>>  /* LOCAL_ALIGNMENT.  */
>>
>> -int
>> -mmix_local_alignment (tree type ATTRIBUTE_UNUSED, int basic_align)
>> +unsigned int
>> +mmix_local_alignment (tree type,
>> +		      enum machine_mode mode ATTRIBUTE_UNUSED,
>> +		      int basic_align)
>>  {
>> -  if (basic_align < 32)
>> +  if (type && basic_align < 32)
>>      return 32;
>>
>>    return basic_align;
>
>Why are you testing type here?
>
>
>Ian