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

[Ian Lance Taylor]

"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