[RFC] Target-specific limits on vector alignment

[RFC] Target-specific limits on vector alignment

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

The ARM ABI states that vectors larger than 64 bits in size still have
64-bit alignment; never-the-less, the HW supports alignment hints of up
to 128-bits in some cases and will trap in a vector has an alignment
that less than the hint.  GCC currently hard-codes larger vectors to be
aligned by the size of the vector, which means that 128-bit vectors are
marked as being 128-bit aligned.

The ARM ABI unfortunately does not support generating such alignment for
parameters passed by value and this can lead to traps at run time.  It
seems that the best way to solve this problem is to allow the back-end
to set an upper limit on the alignment permitted for a vector.

I've implemented this as a separate hook, rather than using the existing
hooks because there's a strong likelihood of breaking some existing ABIs
if I did it another way.

There are a couple of tests that will need some re-working before this
can be committed to deal with the fall-out of making this change; I'll
prepare those changes if this patch is deemed generally acceptable.

R.

	* target.def (TARGET_VECTOR_ALIGNMENT): New hook.
	* doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Likewise.
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_vector_alignment): New function.
	* arm.c (arm_vector_alignment): New function.
	(TARGET_VECTOR_ALIGNMENT): Define.

[-- Attachment #2: vecalign.patch --]
[-- Type: text/plain, Size: 3548 bytes --]

--- config/arm/arm.c	(revision 187425)
+++ config/arm/arm.c	(local)
@@ -259,6 +259,8 @@ static bool arm_array_mode_supported_p (
 					unsigned HOST_WIDE_INT);
 static enum machine_mode arm_preferred_simd_mode (enum machine_mode);
 static bool arm_class_likely_spilled_p (reg_class_t);
+static HOST_WIDE_INT arm_vector_alignment (const_tree type,
+					   HOST_WIDE_INT align);
 static bool arm_vector_alignment_reachable (const_tree type, bool is_packed);
 static bool arm_builtin_support_vector_misalignment (enum machine_mode mode,
 						     const_tree type,
@@ -607,6 +609,9 @@ static const struct attribute_spec arm_a
 #undef TARGET_CLASS_LIKELY_SPILLED_P
 #define TARGET_CLASS_LIKELY_SPILLED_P arm_class_likely_spilled_p
 
+#undef TARGET_VECTOR_ALIGNMENT
+#define TARGET_VECTOR_ALIGNMENT arm_vector_alignment
+
 #undef TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE
 #define TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE \
   arm_vector_alignment_reachable
@@ -24850,6 +25292,15 @@ arm_have_conditional_execution (void)
   return !TARGET_THUMB1;
 }
 
+/* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
+static HOST_WIDE_INT
+arm_vector_alignment (const_tree type ATTRIBUTE_UNUSED, HOST_WIDE_INT align)
+{
+  if (TARGET_AAPCS_BASED)
+    return MIN (align, 64);
+  return align;
+}
+
 static unsigned int
 arm_autovectorize_vector_sizes (void)
 {
--- doc/tm.texi	(revision 187425)
+++ doc/tm.texi	(local)
@@ -1099,6 +1099,12 @@ make it all fit in fewer cache lines.
 If the value of this macro has a type, it should be an unsigned type.
 @end defmac
 
+@deftypefn {Target Hook} HOST_WIDE_INT TARGET_VECTOR_ALIGNMENT (const_tree @var{type}, HOST_WIDE_INT @var{align})
+This hook can be used to limit the alignment for a vector of type
+@var{type}, in order to comply with a platform ABI.  The default is to
+return @var{align}.
+@end deftypefn
+
 @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
 If defined, a C expression to compute the alignment for stack slot.
 @var{type} is the data type, @var{mode} is the widest mode available,
--- doc/tm.texi.in	(revision 187425)
+++ doc/tm.texi.in	(local)
@@ -1087,6 +1087,8 @@ make it all fit in fewer cache lines.
 If the value of this macro has a type, it should be an unsigned type.
 @end defmac
 
+@hook TARGET_VECTOR_ALIGNMENT
+
 @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
 If defined, a C expression to compute the alignment for stack slot.
 @var{type} is the data type, @var{mode} is the widest mode available,
--- target.def	(revision 187425)
+++ target.def	(local)
@@ -1615,6 +1615,14 @@ DEFHOOK
  bool, (enum machine_mode mode),
  hook_bool_mode_false)
 
+DEFHOOK
+(vector_alignment,
+ "This hook can be used to limit the alignment for a vector of type\n\
+@var{type}, in order to comply with a platform ABI.  The default is to\n\
+return @var{align}.",
+ HOST_WIDE_INT, (const_tree type, HOST_WIDE_INT align),
+ default_vector_alignment)
+
 /* True if we should try to use a scalar mode to represent an array,
    overriding the usual MAX_FIXED_MODE limit.  */
 DEFHOOK
--- targhooks.c	(revision 187425)
+++ targhooks.c	(local)
@@ -939,6 +939,13 @@ tree default_mangle_decl_assembler_name 
    return id;
 }
 
+HOST_WIDE_INT
+default_vector_alignment (const_tree type ATTRIBUTE_UNUSED,
+			  HOST_WIDE_INT align)
+{
+  return align;
+}
+
 bool
 default_builtin_vector_alignment_reachable (const_tree type, bool is_packed)
 {

Re: [RFC] Target-specific limits on vector alignment

[Richard Guenther]

On Mon, Jun 11, 2012 at 3:16 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> The ARM ABI states that vectors larger than 64 bits in size still have
> 64-bit alignment; never-the-less, the HW supports alignment hints of up
> to 128-bits in some cases and will trap in a vector has an alignment
> that less than the hint.  GCC currently hard-codes larger vectors to be
> aligned by the size of the vector, which means that 128-bit vectors are
> marked as being 128-bit aligned.
>
> The ARM ABI unfortunately does not support generating such alignment for
> parameters passed by value and this can lead to traps at run time.  It
> seems that the best way to solve this problem is to allow the back-end
> to set an upper limit on the alignment permitted for a vector.
>
> I've implemented this as a separate hook, rather than using the existing
> hooks because there's a strong likelihood of breaking some existing ABIs
> if I did it another way.
>
> There are a couple of tests that will need some re-working before this
> can be committed to deal with the fall-out of making this change; I'll
> prepare those changes if this patch is deemed generally acceptable.

Hm.  Where would you use that target hook?

Richard.

> R.
>
>        * target.def (TARGET_VECTOR_ALIGNMENT): New hook.
>        * doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Likewise.
>        * doc/tm.texi: Regenerate.
>        * targhooks.c (default_vector_alignment): New function.
>        * arm.c (arm_vector_alignment): New function.
>        (TARGET_VECTOR_ALIGNMENT): Define.

Re: [RFC] Target-specific limits on vector alignment

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]

On 11/06/12 15:17, Richard Guenther wrote:
> On Mon, Jun 11, 2012 at 3:16 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>> The ARM ABI states that vectors larger than 64 bits in size still have
>> 64-bit alignment; never-the-less, the HW supports alignment hints of up
>> to 128-bits in some cases and will trap in a vector has an alignment
>> that less than the hint.  GCC currently hard-codes larger vectors to be
>> aligned by the size of the vector, which means that 128-bit vectors are
>> marked as being 128-bit aligned.
>>
>> The ARM ABI unfortunately does not support generating such alignment for
>> parameters passed by value and this can lead to traps at run time.  It
>> seems that the best way to solve this problem is to allow the back-end
>> to set an upper limit on the alignment permitted for a vector.
>>
>> I've implemented this as a separate hook, rather than using the existing
>> hooks because there's a strong likelihood of breaking some existing ABIs
>> if I did it another way.
>>
>> There are a couple of tests that will need some re-working before this
>> can be committed to deal with the fall-out of making this change; I'll
>> prepare those changes if this patch is deemed generally acceptable.
> 
> Hm.  Where would you use that target hook?
> 

Doh!

It was supposed to be in the patch set... :-(

in layout_type(), where the alignment of a vector is forced to the size
of the vector.

R.

[-- Attachment #2: stor.patch --]
[-- Type: text/plain, Size: 720 bytes --]

--- stor-layout.c	(revision 188348)
+++ stor-layout.c	(local)
@@ -2131,9 +2131,11 @@ layout_type (tree type)
 	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
 					    bitsize_int (nunits));
 
-	/* Always naturally align vectors.  This prevents ABI changes
-	   depending on whether or not native vector modes are supported.  */
-	TYPE_ALIGN (type) = tree_low_cst (TYPE_SIZE (type), 0);
+	/* Naturally align vectors, but let the target set an upper
+	   limit.  This prevents ABI changes depending on whether or
+	   not native vector modes are supported.  */
+	TYPE_ALIGN (type)
+	  = targetm.vector_alignment (type, tree_low_cst (TYPE_SIZE (type), 0));
         break;
       }
 

Re: [RFC] Target-specific limits on vector alignment

[Richard Guenther]

On Mon, Jun 11, 2012 at 4:38 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 11/06/12 15:17, Richard Guenther wrote:
>> On Mon, Jun 11, 2012 at 3:16 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>>> The ARM ABI states that vectors larger than 64 bits in size still have
>>> 64-bit alignment; never-the-less, the HW supports alignment hints of up
>>> to 128-bits in some cases and will trap in a vector has an alignment
>>> that less than the hint.  GCC currently hard-codes larger vectors to be
>>> aligned by the size of the vector, which means that 128-bit vectors are
>>> marked as being 128-bit aligned.
>>>
>>> The ARM ABI unfortunately does not support generating such alignment for
>>> parameters passed by value and this can lead to traps at run time.  It
>>> seems that the best way to solve this problem is to allow the back-end
>>> to set an upper limit on the alignment permitted for a vector.
>>>
>>> I've implemented this as a separate hook, rather than using the existing
>>> hooks because there's a strong likelihood of breaking some existing ABIs
>>> if I did it another way.
>>>
>>> There are a couple of tests that will need some re-working before this
>>> can be committed to deal with the fall-out of making this change; I'll
>>> prepare those changes if this patch is deemed generally acceptable.
>>
>> Hm.  Where would you use that target hook?
>>
>
> Doh!
>
> It was supposed to be in the patch set... :-(
>
> in layout_type(), where the alignment of a vector is forced to the size
> of the vector.

Ok.

+	/* Naturally align vectors, but let the target set an upper
+	   limit.  This prevents ABI changes depending on whether or
+	   not native vector modes are supported.  */
+	TYPE_ALIGN (type)
+	  = targetm.vector_alignment (type, tree_low_cst (TYPE_SIZE (type), 0));

The type argument or the size argument looks redundant.  Note that we still
can have such vector "properly" aligned, thus the vectorizer would need to
use build_aligned_type also if it knows the type is aligned, not only when
it thinks it is misaligned.  You basically change the alignment of the "default"
vector type.

Richard.

> R.

Re: [RFC] Target-specific limits on vector alignment

[Richard Earnshaw]

On 11/06/12 15:53, Richard Guenther wrote:
> On Mon, Jun 11, 2012 at 4:38 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>> On 11/06/12 15:17, Richard Guenther wrote:
>>> On Mon, Jun 11, 2012 at 3:16 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>>>> The ARM ABI states that vectors larger than 64 bits in size still have
>>>> 64-bit alignment; never-the-less, the HW supports alignment hints of up
>>>> to 128-bits in some cases and will trap in a vector has an alignment
>>>> that less than the hint.  GCC currently hard-codes larger vectors to be
>>>> aligned by the size of the vector, which means that 128-bit vectors are
>>>> marked as being 128-bit aligned.
>>>>
>>>> The ARM ABI unfortunately does not support generating such alignment for
>>>> parameters passed by value and this can lead to traps at run time.  It
>>>> seems that the best way to solve this problem is to allow the back-end
>>>> to set an upper limit on the alignment permitted for a vector.
>>>>
>>>> I've implemented this as a separate hook, rather than using the existing
>>>> hooks because there's a strong likelihood of breaking some existing ABIs
>>>> if I did it another way.
>>>>
>>>> There are a couple of tests that will need some re-working before this
>>>> can be committed to deal with the fall-out of making this change; I'll
>>>> prepare those changes if this patch is deemed generally acceptable.
>>>
>>> Hm.  Where would you use that target hook?
>>>
>>
>> Doh!
>>
>> It was supposed to be in the patch set... :-(
>>
>> in layout_type(), where the alignment of a vector is forced to the size
>> of the vector.
> 
> Ok.
> 
> +	/* Naturally align vectors, but let the target set an upper
> +	   limit.  This prevents ABI changes depending on whether or
> +	   not native vector modes are supported.  */
> +	TYPE_ALIGN (type)
> +	  = targetm.vector_alignment (type, tree_low_cst (TYPE_SIZE (type), 0));
> 
> The type argument or the size argument looks redundant.  

Technically, yes, we could get rid of "tree_low_cst (TYPE_SIZE (type)"
and calculate it inside the alignment function if it was needed.
However, it seemed likely that most targets would need that number one
way or another, such that passing it would be helpful.

> Note that we still
> can have such vector "properly" aligned, thus the vectorizer would need to
> use build_aligned_type also if it knows the type is aligned, not only when
> it thinks it is misaligned.  You basically change the alignment of the "default"
> vector type.
> 

I'm not sure I follow...

R.

Re: [RFC] Target-specific limits on vector alignment

[Richard Guenther]

On Mon, Jun 11, 2012 at 5:25 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 11/06/12 15:53, Richard Guenther wrote:
>> On Mon, Jun 11, 2012 at 4:38 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>>> On 11/06/12 15:17, Richard Guenther wrote:
>>>> On Mon, Jun 11, 2012 at 3:16 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>>>>> The ARM ABI states that vectors larger than 64 bits in size still have
>>>>> 64-bit alignment; never-the-less, the HW supports alignment hints of up
>>>>> to 128-bits in some cases and will trap in a vector has an alignment
>>>>> that less than the hint.  GCC currently hard-codes larger vectors to be
>>>>> aligned by the size of the vector, which means that 128-bit vectors are
>>>>> marked as being 128-bit aligned.
>>>>>
>>>>> The ARM ABI unfortunately does not support generating such alignment for
>>>>> parameters passed by value and this can lead to traps at run time.  It
>>>>> seems that the best way to solve this problem is to allow the back-end
>>>>> to set an upper limit on the alignment permitted for a vector.
>>>>>
>>>>> I've implemented this as a separate hook, rather than using the existing
>>>>> hooks because there's a strong likelihood of breaking some existing ABIs
>>>>> if I did it another way.
>>>>>
>>>>> There are a couple of tests that will need some re-working before this
>>>>> can be committed to deal with the fall-out of making this change; I'll
>>>>> prepare those changes if this patch is deemed generally acceptable.
>>>>
>>>> Hm.  Where would you use that target hook?
>>>>
>>>
>>> Doh!
>>>
>>> It was supposed to be in the patch set... :-(
>>>
>>> in layout_type(), where the alignment of a vector is forced to the size
>>> of the vector.
>>
>> Ok.
>>
>> +     /* Naturally align vectors, but let the target set an upper
>> +        limit.  This prevents ABI changes depending on whether or
>> +        not native vector modes are supported.  */
>> +     TYPE_ALIGN (type)
>> +       = targetm.vector_alignment (type, tree_low_cst (TYPE_SIZE (type), 0));
>>
>> The type argument or the size argument looks redundant.
>
> Technically, yes, we could get rid of "tree_low_cst (TYPE_SIZE (type)"
> and calculate it inside the alignment function if it was needed.
> However, it seemed likely that most targets would need that number one
> way or another, such that passing it would be helpful.

Well, you don't need it in stor-layout and targets might think the value may
be completely unrelated to the type ...

>> Note that we still
>> can have such vector "properly" aligned, thus the vectorizer would need to
>> use build_aligned_type also if it knows the type is aligned, not only when
>> it thinks it is misaligned.  You basically change the alignment of the "default"
>> vector type.
>>
>
> I'm not sure I follow...

I say that a large vector may be still aligned, so the vectorizer when creating
vector memory references has to use a non-default aligned vector type when
the vector is aligned.  It won't do that at the moment.

Richard.

> R.
>
>
>

[PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Guenther wrote:
> On Mon, Jun 11, 2012 at 5:25 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> > On 11/06/12 15:53, Richard Guenther wrote:
> >> The type argument or the size argument looks redundant.
> >
> > Technically, yes, we could get rid of "tree_low_cst (TYPE_SIZE (type)"
> > and calculate it inside the alignment function if it was needed.
> > However, it seemed likely that most targets would need that number one
> > way or another, such that passing it would be helpful.
> 
> Well, you don't need it in stor-layout and targets might think the value
> may be completely unrelated to the type ...
> 
> >> Note that we still can have such vector "properly" aligned, thus the
> >> vectorizer would need to use build_aligned_type also if it knows the
> >> type is aligned, not only when thinks it is misaligned.  You basically
> >> change the alignment of the "default" vector type.
> >
> > I'm not sure I follow...
> 
> I say that a large vector may be still aligned, so the vectorizer when
> creating vector memory references has to use a non-default aligned vector
> type when the vector is aligned.  It won't do that at the moment.

Richard (Earnshaw) has asked me to take over working on this patch now.

I've now made the change requested above and removed the size argument.
The target is now simply asked to return the required alignment for the
given vector type.  I've also added a check for the case where the
target provides both an alignment and a mode for a vector type, but
the mode actually requires bigger alignment than the type.  This is
simply rejected (the target can fix this by reporting a different
type alignment or changing the mode alignment).

I've not made any attempts to have the vectorizer register larger
alignments than the one returned by the target hook.  It's not
clear to me when this would be useful (at least on ARM) ...

I've also run the testsuite, and this actually uncovered to bugs in
the vectorizer where it made an implicit assumption that vector types
must always be naturally aligned:

- In vect_update_misalignment_for_peel, the code used the vector size
  instead of the required alignment in order to bound misalignment
  values -- leading to a misalignment value bigger than the underlying
  alignment requirement of the vector type, causing an ICE later on

- In vect_do_peeling_for_loop_bound, the code divided the vector type
  alignment by the number of elements in order to arrive at the element
  size ... this returns a wrong value if the alignment is less than the
  vector size, causing incorrect code to be generated

  (This routine also had some confusion between size and alignment in
  comments and variable names, which I've fixed as well.)

Finally, two test cases still failed spuriously:

- gcc.dg/align-2.c actually checked that vector types are naturally
  aligned

- gcc.dg/vect/slp-25.c checked that we needed to perform peeling for
  alignment, which we actually don't need any more if vector types
  have a lesser alignment requirement in the first place

I've added a new effective target flag to check whether the target
requires natural alignment for vector types, and disabled those two
tests if it doesn't.

With those changes, I've completed testing with no regressions on
arm-linux-gnueabi.

OK for mainline?

Bye,
Ulrich


ChangeLog:

	* target.def (vector_alignment): New target hook.
	* doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_vector_alignment): New function.
	* targhooks.h (default_vector_alignment): Add prototype.
	* stor-layout.c (layout_type): Use targetm.vector_alignment.
	* config/arm/arm.c (arm_vector_alignment): New function.
	(TARGET_VECTOR_ALIGNMENT): Define.

	* tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
	vector type alignment instead of size.
	* tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
	element type size directly instead of computing it from alignment.
	Fix variable naming and comment.

testsuite/ChangeLog:

	* lib/target-supports.exp
	(check_effective_target_vect_natural_alignment): New function.
	* gcc.dg/align-2.c: Only run on targets with natural alignment
	of vector types.
	* gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
	alignment of vector types.


Index: gcc/target.def
===================================================================
*** gcc/target.def	(revision 189809)
--- gcc/target.def	(working copy)
*************** DEFHOOK
*** 1659,1664 ****
--- 1659,1672 ----
   bool, (enum machine_mode mode),
   hook_bool_mode_false)
  
+ DEFHOOK
+ (vector_alignment,
+  "This hook can be used to define the alignment for a vector of type\n\
+ @var{type}, in order to comply with a platform ABI.  The default is to\n\
+ require natural alignment for vector types.",
+  HOST_WIDE_INT, (const_tree type),
+  default_vector_alignment)
+ 
  /* True if we should try to use a scalar mode to represent an array,
     overriding the usual MAX_FIXED_MODE limit.  */
  DEFHOOK
Index: gcc/doc/tm.texi
===================================================================
*** gcc/doc/tm.texi	(revision 189809)
--- gcc/doc/tm.texi	(working copy)
*************** make it all fit in fewer cache lines.
*** 1107,1112 ****
--- 1107,1118 ----
  If the value of this macro has a type, it should be an unsigned type.
  @end defmac
  
+ @deftypefn {Target Hook} HOST_WIDE_INT TARGET_VECTOR_ALIGNMENT (const_tree @var{type})
+ This hook can be used to define the alignment for a vector of type
+ @var{type}, in order to comply with a platform ABI.  The default is to
+ require natural alignment for vector types.
+ @end deftypefn
+ 
  @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
  If defined, a C expression to compute the alignment for stack slot.
  @var{type} is the data type, @var{mode} is the widest mode available,
Index: gcc/doc/tm.texi.in
===================================================================
*** gcc/doc/tm.texi.in	(revision 189809)
--- gcc/doc/tm.texi.in	(working copy)
*************** make it all fit in fewer cache lines.
*** 1091,1096 ****
--- 1091,1098 ----
  If the value of this macro has a type, it should be an unsigned type.
  @end defmac
  
+ @hook TARGET_VECTOR_ALIGNMENT
+ 
  @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
  If defined, a C expression to compute the alignment for stack slot.
  @var{type} is the data type, @var{mode} is the widest mode available,
Index: gcc/targhooks.c
===================================================================
*** gcc/targhooks.c	(revision 189809)
--- gcc/targhooks.c	(working copy)
*************** tree default_mangle_decl_assembler_name 
*** 945,950 ****
--- 945,957 ----
     return id;
  }
  
+ /* Default to natural alignment for vector types.  */
+ HOST_WIDE_INT
+ default_vector_alignment (const_tree type)
+ {
+   return tree_low_cst (TYPE_SIZE (type), 0);
+ }
+ 
  bool
  default_builtin_vector_alignment_reachable (const_tree type, bool is_packed)
  {
Index: gcc/targhooks.h
===================================================================
*** gcc/targhooks.h	(revision 189809)
--- gcc/targhooks.h	(working copy)
*************** extern int default_builtin_vectorization
*** 83,88 ****
--- 83,90 ----
  
  extern tree default_builtin_reciprocal (unsigned int, bool, bool);
  
+ extern HOST_WIDE_INT default_vector_alignment (const_tree);
+ 
  extern bool default_builtin_vector_alignment_reachable (const_tree, bool);
  extern bool
  default_builtin_support_vector_misalignment (enum machine_mode mode,
Index: gcc/stor-layout.c
===================================================================
*** gcc/stor-layout.c	(revision 189809)
--- gcc/stor-layout.c	(working copy)
*************** layout_type (tree type)
*** 2131,2139 ****
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
  					    bitsize_int (nunits));
  
! 	/* Always naturally align vectors.  This prevents ABI changes
! 	   depending on whether or not native vector modes are supported.  */
! 	TYPE_ALIGN (type) = tree_low_cst (TYPE_SIZE (type), 0);
          break;
        }
  
--- 2131,2147 ----
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
  					    bitsize_int (nunits));
  
! 	/* For vector types, we do not default to the mode's alignment.
! 	   Instead, query a target hook, defaulting to natural alignment.
! 	   This prevents ABI changes depending on whether or not native
! 	   vector modes are supported.  */
! 	TYPE_ALIGN (type) = targetm.vector_alignment (type);
! 
! 	/* However, if the underlying mode requires a bigger alignment than
! 	   what the target hook provides, we cannot use the mode.  For now,
! 	   simply reject that case.  */
! 	gcc_assert (TYPE_ALIGN (type)
! 		    >= GET_MODE_ALIGNMENT (TYPE_MODE (type)));
          break;
        }
  
Index: gcc/config/arm/arm.c
===================================================================
*** gcc/config/arm/arm.c	(revision 189809)
--- gcc/config/arm/arm.c	(working copy)
*************** static bool arm_array_mode_supported_p (
*** 254,259 ****
--- 254,260 ----
  					unsigned HOST_WIDE_INT);
  static enum machine_mode arm_preferred_simd_mode (enum machine_mode);
  static bool arm_class_likely_spilled_p (reg_class_t);
+ static HOST_WIDE_INT arm_vector_alignment (const_tree type);
  static bool arm_vector_alignment_reachable (const_tree type, bool is_packed);
  static bool arm_builtin_support_vector_misalignment (enum machine_mode mode,
  						     const_tree type,
*************** static const struct attribute_spec arm_a
*** 602,607 ****
--- 603,611 ----
  #undef TARGET_CLASS_LIKELY_SPILLED_P
  #define TARGET_CLASS_LIKELY_SPILLED_P arm_class_likely_spilled_p
  
+ #undef TARGET_VECTOR_ALIGNMENT
+ #define TARGET_VECTOR_ALIGNMENT arm_vector_alignment
+ 
  #undef TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE
  #define TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE \
    arm_vector_alignment_reachable
*************** arm_have_conditional_execution (void)
*** 25051,25056 ****
--- 25055,25072 ----
    return !TARGET_THUMB1;
  }
  
+ /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
+ static HOST_WIDE_INT
+ arm_vector_alignment (const_tree type)
+ {
+   HOST_WIDE_INT align = tree_low_cst (TYPE_SIZE (type), 0);
+ 
+   if (TARGET_AAPCS_BASED)
+     align = MIN (align, 64);
+ 
+   return align;
+ }
+ 
  static unsigned int
  arm_autovectorize_vector_sizes (void)
  {
Index: gcc/tree-vect-data-refs.c
===================================================================
*** gcc/tree-vect-data-refs.c	(revision 189809)
--- gcc/tree-vect-data-refs.c	(working copy)
*************** vect_update_misalignment_for_peel (struc
*** 1059,1065 ****
        int misal = DR_MISALIGNMENT (dr);
        tree vectype = STMT_VINFO_VECTYPE (stmt_info);
        misal += negative ? -npeel * dr_size : npeel * dr_size;
!       misal &= GET_MODE_SIZE (TYPE_MODE (vectype)) - 1;
        SET_DR_MISALIGNMENT (dr, misal);
        return;
      }
--- 1059,1065 ----
        int misal = DR_MISALIGNMENT (dr);
        tree vectype = STMT_VINFO_VECTYPE (stmt_info);
        misal += negative ? -npeel * dr_size : npeel * dr_size;
!       misal &= (TYPE_ALIGN (vectype) / BITS_PER_UNIT) - 1;
        SET_DR_MISALIGNMENT (dr, misal);
        return;
      }
Index: gcc/tree-vect-loop-manip.c
===================================================================
*** gcc/tree-vect-loop-manip.c	(revision 189809)
--- gcc/tree-vect-loop-manip.c	(working copy)
*************** vect_do_peeling_for_loop_bound (loop_vec
*** 1937,1943 ****
     If the misalignment of DR is known at compile time:
       addr_mis = int mis = DR_MISALIGNMENT (dr);
     Else, compute address misalignment in bytes:
!      addr_mis = addr & (vectype_size - 1)
  
     prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
  
--- 1937,1943 ----
     If the misalignment of DR is known at compile time:
       addr_mis = int mis = DR_MISALIGNMENT (dr);
     Else, compute address misalignment in bytes:
!      addr_mis = addr & (vectype_align - 1)
  
     prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
  
*************** vect_gen_niters_for_prolog_loop (loop_ve
*** 1991,1999 ****
        tree start_addr = vect_create_addr_base_for_vector_ref (dr_stmt,
  						&new_stmts, offset, loop);
        tree type = unsigned_type_for (TREE_TYPE (start_addr));
!       tree vectype_size_minus_1 = build_int_cst (type, vectype_align - 1);
!       tree elem_size_log =
!         build_int_cst (type, exact_log2 (vectype_align/nelements));
        tree nelements_minus_1 = build_int_cst (type, nelements - 1);
        tree nelements_tree = build_int_cst (type, nelements);
        tree byte_misalign;
--- 1991,2000 ----
        tree start_addr = vect_create_addr_base_for_vector_ref (dr_stmt,
  						&new_stmts, offset, loop);
        tree type = unsigned_type_for (TREE_TYPE (start_addr));
!       tree vectype_align_minus_1 = build_int_cst (type, vectype_align - 1);
!       HOST_WIDE_INT elem_size =
!                 int_cst_value (TYPE_SIZE_UNIT (TREE_TYPE (vectype)));
!       tree elem_size_log = build_int_cst (type, exact_log2 (elem_size));
        tree nelements_minus_1 = build_int_cst (type, nelements - 1);
        tree nelements_tree = build_int_cst (type, nelements);
        tree byte_misalign;
*************** vect_gen_niters_for_prolog_loop (loop_ve
*** 2002,2011 ****
        new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
        gcc_assert (!new_bb);
  
!       /* Create:  byte_misalign = addr & (vectype_size - 1)  */
        byte_misalign =
          fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr), 
!                      vectype_size_minus_1);
  
        /* Create:  elem_misalign = byte_misalign / element_size  */
        elem_misalign =
--- 2003,2012 ----
        new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
        gcc_assert (!new_bb);
  
!       /* Create:  byte_misalign = addr & (vectype_align - 1)  */
        byte_misalign =
          fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr), 
!                      vectype_align_minus_1);
  
        /* Create:  elem_misalign = byte_misalign / element_size  */
        elem_misalign =
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
*** gcc/testsuite/lib/target-supports.exp	(revision 189809)
--- gcc/testsuite/lib/target-supports.exp	(working copy)
*************** proc check_effective_target_natural_alig
*** 3379,3384 ****
--- 3379,3404 ----
      return $et_natural_alignment_64_saved
  }
  
+ # Return 1 if all vector types are naturally aligned (aligned to their
+ # type-size), 0 otherwise.
+ #
+ # This won't change for different subtargets so cache the result.
+ 
+ proc check_effective_target_vect_natural_alignment { } {
+     global et_vect_natural_alignment
+ 
+     if [info exists et_vect_natural_alignment_saved] {
+         verbose "check_effective_target_vect_natural_alignment: using cached result" 2
+     } else {
+         set et_vect_natural_alignment_saved 1
+         if { [check_effective_target_arm_eabi] } {
+             set et_vect_natural_alignment_saved 0
+         }
+     }
+     verbose "check_effective_target_vect_natural_alignment: returning $et_vect_natural_alignment_saved" 2
+     return $et_vect_natural_alignment_saved
+ }
+ 
  # Return 1 if vector alignment (for types of size 32 bit or less) is reachable, 0 otherwise.
  #
  # This won't change for different subtargets so cache the result.
Index: gcc/testsuite/gcc.dg/align-2.c
===================================================================
*** gcc/testsuite/gcc.dg/align-2.c	(revision 189809)
--- gcc/testsuite/gcc.dg/align-2.c	(working copy)
***************
*** 1,5 ****
  /* PR 17962 */
! /* { dg-do compile } */
  /* { dg-options "" } */
  
  typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
--- 1,5 ----
  /* PR 17962 */
! /* { dg-do compile { target vect_natural_alignment } } */
  /* { dg-options "" } */
  
  typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
Index: gcc/testsuite/gcc.dg/vect/slp-25.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/slp-25.c	(revision 189809)
--- gcc/testsuite/gcc.dg/vect/slp-25.c	(working copy)
*************** int main (void)
*** 56,60 ****
  
  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 56,60 ----
  
  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align || { ! vect_natural_alignment } } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH v2] Target-specific limits on vector alignment

[Hans-Peter Nilsson]

> From: Ulrich Weigand <uweigand@de.ibm.com>
> Date: Fri, 27 Jul 2012 17:24:08 +0200

> Richard (Earnshaw) has asked me to take over working on this patch now.
> 
> I've now made the change requested above and removed the size argument.
> The target is now simply asked to return the required alignment for the
> given vector type.  I've also added a check for the case where the
> target provides both an alignment and a mode for a vector type, but
> the mode actually requires bigger alignment than the type.  This is
> simply rejected (the target can fix this by reporting a different
> type alignment or changing the mode alignment).

Sounds great, IMHO.

brgds, H-P

Re: [PATCH v2] Target-specific limits on vector alignment

[Richard Guenther]

On Fri, Jul 27, 2012 at 5:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Richard Guenther wrote:
>> On Mon, Jun 11, 2012 at 5:25 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>> > On 11/06/12 15:53, Richard Guenther wrote:
>> >> The type argument or the size argument looks redundant.
>> >
>> > Technically, yes, we could get rid of "tree_low_cst (TYPE_SIZE (type)"
>> > and calculate it inside the alignment function if it was needed.
>> > However, it seemed likely that most targets would need that number one
>> > way or another, such that passing it would be helpful.
>>
>> Well, you don't need it in stor-layout and targets might think the value
>> may be completely unrelated to the type ...
>>
>> >> Note that we still can have such vector "properly" aligned, thus the
>> >> vectorizer would need to use build_aligned_type also if it knows the
>> >> type is aligned, not only when thinks it is misaligned.  You basically
>> >> change the alignment of the "default" vector type.
>> >
>> > I'm not sure I follow...
>>
>> I say that a large vector may be still aligned, so the vectorizer when
>> creating vector memory references has to use a non-default aligned vector
>> type when the vector is aligned.  It won't do that at the moment.
>
> Richard (Earnshaw) has asked me to take over working on this patch now.
>
> I've now made the change requested above and removed the size argument.
> The target is now simply asked to return the required alignment for the
> given vector type.  I've also added a check for the case where the
> target provides both an alignment and a mode for a vector type, but
> the mode actually requires bigger alignment than the type.  This is
> simply rejected (the target can fix this by reporting a different
> type alignment or changing the mode alignment).
>
> I've not made any attempts to have the vectorizer register larger
> alignments than the one returned by the target hook.  It's not
> clear to me when this would be useful (at least on ARM) ...
>
> I've also run the testsuite, and this actually uncovered to bugs in
> the vectorizer where it made an implicit assumption that vector types
> must always be naturally aligned:
>
> - In vect_update_misalignment_for_peel, the code used the vector size
>   instead of the required alignment in order to bound misalignment
>   values -- leading to a misalignment value bigger than the underlying
>   alignment requirement of the vector type, causing an ICE later on
>
> - In vect_do_peeling_for_loop_bound, the code divided the vector type
>   alignment by the number of elements in order to arrive at the element
>   size ... this returns a wrong value if the alignment is less than the
>   vector size, causing incorrect code to be generated
>
>   (This routine also had some confusion between size and alignment in
>   comments and variable names, which I've fixed as well.)
>
> Finally, two test cases still failed spuriously:
>
> - gcc.dg/align-2.c actually checked that vector types are naturally
>   aligned
>
> - gcc.dg/vect/slp-25.c checked that we needed to perform peeling for
>   alignment, which we actually don't need any more if vector types
>   have a lesser alignment requirement in the first place
>
> I've added a new effective target flag to check whether the target
> requires natural alignment for vector types, and disabled those two
> tests if it doesn't.
>
> With those changes, I've completed testing with no regressions on
> arm-linux-gnueabi.
>
> OK for mainline?

Ok.  Please add to the documentation that the default vector alignment
has to be a power-of-two multiple of the default vector element alignment.
You probably want to double-check vector_alignment_reachable_p as well
which checks whether vector alignment can be reached by peeling off
scalar iterations.

Thanks,
Richard.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
>         * target.def (vector_alignment): New target hook.
>         * doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
>         * doc/tm.texi: Regenerate.
>         * targhooks.c (default_vector_alignment): New function.
>         * targhooks.h (default_vector_alignment): Add prototype.
>         * stor-layout.c (layout_type): Use targetm.vector_alignment.
>         * config/arm/arm.c (arm_vector_alignment): New function.
>         (TARGET_VECTOR_ALIGNMENT): Define.
>
>         * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
>         vector type alignment instead of size.
>         * tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
>         element type size directly instead of computing it from alignment.
>         Fix variable naming and comment.
>
> testsuite/ChangeLog:
>
>         * lib/target-supports.exp
>         (check_effective_target_vect_natural_alignment): New function.
>         * gcc.dg/align-2.c: Only run on targets with natural alignment
>         of vector types.
>         * gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
>         alignment of vector types.
>
>
> Index: gcc/target.def
> ===================================================================
> *** gcc/target.def      (revision 189809)
> --- gcc/target.def      (working copy)
> *************** DEFHOOK
> *** 1659,1664 ****
> --- 1659,1672 ----
>    bool, (enum machine_mode mode),
>    hook_bool_mode_false)
>
> + DEFHOOK
> + (vector_alignment,
> +  "This hook can be used to define the alignment for a vector of type\n\
> + @var{type}, in order to comply with a platform ABI.  The default is to\n\
> + require natural alignment for vector types.",
> +  HOST_WIDE_INT, (const_tree type),
> +  default_vector_alignment)
> +
>   /* True if we should try to use a scalar mode to represent an array,
>      overriding the usual MAX_FIXED_MODE limit.  */
>   DEFHOOK
> Index: gcc/doc/tm.texi
> ===================================================================
> *** gcc/doc/tm.texi     (revision 189809)
> --- gcc/doc/tm.texi     (working copy)
> *************** make it all fit in fewer cache lines.
> *** 1107,1112 ****
> --- 1107,1118 ----
>   If the value of this macro has a type, it should be an unsigned type.
>   @end defmac
>
> + @deftypefn {Target Hook} HOST_WIDE_INT TARGET_VECTOR_ALIGNMENT (const_tree @var{type})
> + This hook can be used to define the alignment for a vector of type
> + @var{type}, in order to comply with a platform ABI.  The default is to
> + require natural alignment for vector types.
> + @end deftypefn
> +
>   @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
>   If defined, a C expression to compute the alignment for stack slot.
>   @var{type} is the data type, @var{mode} is the widest mode available,
> Index: gcc/doc/tm.texi.in
> ===================================================================
> *** gcc/doc/tm.texi.in  (revision 189809)
> --- gcc/doc/tm.texi.in  (working copy)
> *************** make it all fit in fewer cache lines.
> *** 1091,1096 ****
> --- 1091,1098 ----
>   If the value of this macro has a type, it should be an unsigned type.
>   @end defmac
>
> + @hook TARGET_VECTOR_ALIGNMENT
> +
>   @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
>   If defined, a C expression to compute the alignment for stack slot.
>   @var{type} is the data type, @var{mode} is the widest mode available,
> Index: gcc/targhooks.c
> ===================================================================
> *** gcc/targhooks.c     (revision 189809)
> --- gcc/targhooks.c     (working copy)
> *************** tree default_mangle_decl_assembler_name
> *** 945,950 ****
> --- 945,957 ----
>      return id;
>   }
>
> + /* Default to natural alignment for vector types.  */
> + HOST_WIDE_INT
> + default_vector_alignment (const_tree type)
> + {
> +   return tree_low_cst (TYPE_SIZE (type), 0);
> + }
> +
>   bool
>   default_builtin_vector_alignment_reachable (const_tree type, bool is_packed)
>   {
> Index: gcc/targhooks.h
> ===================================================================
> *** gcc/targhooks.h     (revision 189809)
> --- gcc/targhooks.h     (working copy)
> *************** extern int default_builtin_vectorization
> *** 83,88 ****
> --- 83,90 ----
>
>   extern tree default_builtin_reciprocal (unsigned int, bool, bool);
>
> + extern HOST_WIDE_INT default_vector_alignment (const_tree);
> +
>   extern bool default_builtin_vector_alignment_reachable (const_tree, bool);
>   extern bool
>   default_builtin_support_vector_misalignment (enum machine_mode mode,
> Index: gcc/stor-layout.c
> ===================================================================
> *** gcc/stor-layout.c   (revision 189809)
> --- gcc/stor-layout.c   (working copy)
> *************** layout_type (tree type)
> *** 2131,2139 ****
>         TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
>                                             bitsize_int (nunits));
>
> !       /* Always naturally align vectors.  This prevents ABI changes
> !          depending on whether or not native vector modes are supported.  */
> !       TYPE_ALIGN (type) = tree_low_cst (TYPE_SIZE (type), 0);
>           break;
>         }
>
> --- 2131,2147 ----
>         TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
>                                             bitsize_int (nunits));
>
> !       /* For vector types, we do not default to the mode's alignment.
> !          Instead, query a target hook, defaulting to natural alignment.
> !          This prevents ABI changes depending on whether or not native
> !          vector modes are supported.  */
> !       TYPE_ALIGN (type) = targetm.vector_alignment (type);
> !
> !       /* However, if the underlying mode requires a bigger alignment than
> !          what the target hook provides, we cannot use the mode.  For now,
> !          simply reject that case.  */
> !       gcc_assert (TYPE_ALIGN (type)
> !                   >= GET_MODE_ALIGNMENT (TYPE_MODE (type)));
>           break;
>         }
>
> Index: gcc/config/arm/arm.c
> ===================================================================
> *** gcc/config/arm/arm.c        (revision 189809)
> --- gcc/config/arm/arm.c        (working copy)
> *************** static bool arm_array_mode_supported_p (
> *** 254,259 ****
> --- 254,260 ----
>                                         unsigned HOST_WIDE_INT);
>   static enum machine_mode arm_preferred_simd_mode (enum machine_mode);
>   static bool arm_class_likely_spilled_p (reg_class_t);
> + static HOST_WIDE_INT arm_vector_alignment (const_tree type);
>   static bool arm_vector_alignment_reachable (const_tree type, bool is_packed);
>   static bool arm_builtin_support_vector_misalignment (enum machine_mode mode,
>                                                      const_tree type,
> *************** static const struct attribute_spec arm_a
> *** 602,607 ****
> --- 603,611 ----
>   #undef TARGET_CLASS_LIKELY_SPILLED_P
>   #define TARGET_CLASS_LIKELY_SPILLED_P arm_class_likely_spilled_p
>
> + #undef TARGET_VECTOR_ALIGNMENT
> + #define TARGET_VECTOR_ALIGNMENT arm_vector_alignment
> +
>   #undef TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE
>   #define TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE \
>     arm_vector_alignment_reachable
> *************** arm_have_conditional_execution (void)
> *** 25051,25056 ****
> --- 25055,25072 ----
>     return !TARGET_THUMB1;
>   }
>
> + /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
> + static HOST_WIDE_INT
> + arm_vector_alignment (const_tree type)
> + {
> +   HOST_WIDE_INT align = tree_low_cst (TYPE_SIZE (type), 0);
> +
> +   if (TARGET_AAPCS_BASED)
> +     align = MIN (align, 64);
> +
> +   return align;
> + }
> +
>   static unsigned int
>   arm_autovectorize_vector_sizes (void)
>   {
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> *** gcc/tree-vect-data-refs.c   (revision 189809)
> --- gcc/tree-vect-data-refs.c   (working copy)
> *************** vect_update_misalignment_for_peel (struc
> *** 1059,1065 ****
>         int misal = DR_MISALIGNMENT (dr);
>         tree vectype = STMT_VINFO_VECTYPE (stmt_info);
>         misal += negative ? -npeel * dr_size : npeel * dr_size;
> !       misal &= GET_MODE_SIZE (TYPE_MODE (vectype)) - 1;
>         SET_DR_MISALIGNMENT (dr, misal);
>         return;
>       }
> --- 1059,1065 ----
>         int misal = DR_MISALIGNMENT (dr);
>         tree vectype = STMT_VINFO_VECTYPE (stmt_info);
>         misal += negative ? -npeel * dr_size : npeel * dr_size;
> !       misal &= (TYPE_ALIGN (vectype) / BITS_PER_UNIT) - 1;
>         SET_DR_MISALIGNMENT (dr, misal);
>         return;
>       }
> Index: gcc/tree-vect-loop-manip.c
> ===================================================================
> *** gcc/tree-vect-loop-manip.c  (revision 189809)
> --- gcc/tree-vect-loop-manip.c  (working copy)
> *************** vect_do_peeling_for_loop_bound (loop_vec
> *** 1937,1943 ****
>      If the misalignment of DR is known at compile time:
>        addr_mis = int mis = DR_MISALIGNMENT (dr);
>      Else, compute address misalignment in bytes:
> !      addr_mis = addr & (vectype_size - 1)
>
>      prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
>
> --- 1937,1943 ----
>      If the misalignment of DR is known at compile time:
>        addr_mis = int mis = DR_MISALIGNMENT (dr);
>      Else, compute address misalignment in bytes:
> !      addr_mis = addr & (vectype_align - 1)
>
>      prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
>
> *************** vect_gen_niters_for_prolog_loop (loop_ve
> *** 1991,1999 ****
>         tree start_addr = vect_create_addr_base_for_vector_ref (dr_stmt,
>                                                 &new_stmts, offset, loop);
>         tree type = unsigned_type_for (TREE_TYPE (start_addr));
> !       tree vectype_size_minus_1 = build_int_cst (type, vectype_align - 1);
> !       tree elem_size_log =
> !         build_int_cst (type, exact_log2 (vectype_align/nelements));
>         tree nelements_minus_1 = build_int_cst (type, nelements - 1);
>         tree nelements_tree = build_int_cst (type, nelements);
>         tree byte_misalign;
> --- 1991,2000 ----
>         tree start_addr = vect_create_addr_base_for_vector_ref (dr_stmt,
>                                                 &new_stmts, offset, loop);
>         tree type = unsigned_type_for (TREE_TYPE (start_addr));
> !       tree vectype_align_minus_1 = build_int_cst (type, vectype_align - 1);
> !       HOST_WIDE_INT elem_size =
> !                 int_cst_value (TYPE_SIZE_UNIT (TREE_TYPE (vectype)));
> !       tree elem_size_log = build_int_cst (type, exact_log2 (elem_size));
>         tree nelements_minus_1 = build_int_cst (type, nelements - 1);
>         tree nelements_tree = build_int_cst (type, nelements);
>         tree byte_misalign;
> *************** vect_gen_niters_for_prolog_loop (loop_ve
> *** 2002,2011 ****
>         new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
>         gcc_assert (!new_bb);
>
> !       /* Create:  byte_misalign = addr & (vectype_size - 1)  */
>         byte_misalign =
>           fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr),
> !                      vectype_size_minus_1);
>
>         /* Create:  elem_misalign = byte_misalign / element_size  */
>         elem_misalign =
> --- 2003,2012 ----
>         new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
>         gcc_assert (!new_bb);
>
> !       /* Create:  byte_misalign = addr & (vectype_align - 1)  */
>         byte_misalign =
>           fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr),
> !                      vectype_align_minus_1);
>
>         /* Create:  elem_misalign = byte_misalign / element_size  */
>         elem_misalign =
> Index: gcc/testsuite/lib/target-supports.exp
> ===================================================================
> *** gcc/testsuite/lib/target-supports.exp       (revision 189809)
> --- gcc/testsuite/lib/target-supports.exp       (working copy)
> *************** proc check_effective_target_natural_alig
> *** 3379,3384 ****
> --- 3379,3404 ----
>       return $et_natural_alignment_64_saved
>   }
>
> + # Return 1 if all vector types are naturally aligned (aligned to their
> + # type-size), 0 otherwise.
> + #
> + # This won't change for different subtargets so cache the result.
> +
> + proc check_effective_target_vect_natural_alignment { } {
> +     global et_vect_natural_alignment
> +
> +     if [info exists et_vect_natural_alignment_saved] {
> +         verbose "check_effective_target_vect_natural_alignment: using cached result" 2
> +     } else {
> +         set et_vect_natural_alignment_saved 1
> +         if { [check_effective_target_arm_eabi] } {
> +             set et_vect_natural_alignment_saved 0
> +         }
> +     }
> +     verbose "check_effective_target_vect_natural_alignment: returning $et_vect_natural_alignment_saved" 2
> +     return $et_vect_natural_alignment_saved
> + }
> +
>   # Return 1 if vector alignment (for types of size 32 bit or less) is reachable, 0 otherwise.
>   #
>   # This won't change for different subtargets so cache the result.
> Index: gcc/testsuite/gcc.dg/align-2.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/align-2.c      (revision 189809)
> --- gcc/testsuite/gcc.dg/align-2.c      (working copy)
> ***************
> *** 1,5 ****
>   /* PR 17962 */
> ! /* { dg-do compile } */
>   /* { dg-options "" } */
>
>   typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
> --- 1,5 ----
>   /* PR 17962 */
> ! /* { dg-do compile { target vect_natural_alignment } } */
>   /* { dg-options "" } */
>
>   typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
> Index: gcc/testsuite/gcc.dg/vect/slp-25.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/vect/slp-25.c  (revision 189809)
> --- gcc/testsuite/gcc.dg/vect/slp-25.c  (working copy)
> *************** int main (void)
> *** 56,60 ****
>
>   /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
>   /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
> ! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align } } } } */
>   /* { dg-final { cleanup-tree-dump "vect" } } */
> --- 56,60 ----
>
>   /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
>   /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
> ! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align || { ! vect_natural_alignment } } } } } */
>   /* { dg-final { cleanup-tree-dump "vect" } } */
>
> --
>   Dr. Ulrich Weigand
>   GNU Toolchain for Linux on System z and Cell BE
>   Ulrich.Weigand@de.ibm.com
>

Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Guenther wrote:
> On Fri, Jul 27, 2012 at 5:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > OK for mainline?
> 
> Ok.  Please add to the documentation that the default vector alignment
> has to be a power-of-two multiple of the default vector element alignment.

Committed, thanks.  The documentation now reads:

+  "This hook can be used to define the alignment for a vector of type\n\
+ @var{type}, in order to comply with a platform ABI.  The default is to\n\
+ require natural alignment for vector types.  The alignment returned by\n\
+ this hook must be a power-of-two multiple of the default alignment of\n\
+ the vector element type.",

> You probably want to double-check vector_alignment_reachable_p as well
> which checks whether vector alignment can be reached by peeling off
> scalar iterations.

I've looked at the ARM implementation, and it still seems to correct
(and efficient) with vector alignment change (basically, unless the
element type is packed, everything is reachable).

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[RFA 4.7/4.6] Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Guenther wrote:
> On Fri, Jul 27, 2012 at 5:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > ChangeLog:
> >
> >         * target.def (vector_alignment): New target hook.
> >         * doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
> >         * doc/tm.texi: Regenerate.
> >         * targhooks.c (default_vector_alignment): New function.
> >         * targhooks.h (default_vector_alignment): Add prototype.
> >         * stor-layout.c (layout_type): Use targetm.vector_alignment.
> >         * config/arm/arm.c (arm_vector_alignment): New function.
> >         (TARGET_VECTOR_ALIGNMENT): Define.
> >
> >         * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
> >         vector type alignment instead of size.
> >         * tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
> >         element type size directly instead of computing it from alignment.
> >         Fix variable naming and comment.
> >
> > testsuite/ChangeLog:
> >
> >         * lib/target-supports.exp
> >         (check_effective_target_vect_natural_alignment): New function.
> >         * gcc.dg/align-2.c: Only run on targets with natural alignment
> >         of vector types.
> >         * gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
> >         alignment of vector types.


> > OK for mainline?
> 
> Ok.

Would it be OK to backport this to 4.7 and possibly 4.6?

This patch represents a change in the ABI on ARM (the alignment could
potentially affect struct member offsets, for example), which could
conceivably cause incompatibilities with code compiled with older
versions of GCC.  We had originally decided we nevertheless want to
implement this change on ARM, because:

- GCC is now in compliance with the platform ABI document
- the old code could actually be buggy since GCC *assumed* 16-byte
  alignment that wasn't actually guaranteed
- code actually affected by this change ought to be rare (code using
  NEON vector types is rare in the first place, code using *structure
  members* of such types is even rarer, and code using such structures
  in cross-module APIs seems to be extremely rare)

Nevertheless, given we do want to make this change, it would be best
to roll it out as quickly as possible, to minimize the time period
where people might use old (not yet fixed) compilers to generate
non-ABI-compliant binaries.  Thus, we think it would be good for
the change to be applied to all still open release branches as well.

(Note that while the patch contains changes to common code, those
should be no-ops for all targets that do not implement the new hook.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFA 4.7/4.6] Re: [PATCH v2] Target-specific limits on vector alignment

[Richard Guenther]

On Tue, Aug 7, 2012 at 4:56 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Richard Guenther wrote:
>> On Fri, Jul 27, 2012 at 5:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>> > ChangeLog:
>> >
>> >         * target.def (vector_alignment): New target hook.
>> >         * doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
>> >         * doc/tm.texi: Regenerate.
>> >         * targhooks.c (default_vector_alignment): New function.
>> >         * targhooks.h (default_vector_alignment): Add prototype.
>> >         * stor-layout.c (layout_type): Use targetm.vector_alignment.
>> >         * config/arm/arm.c (arm_vector_alignment): New function.
>> >         (TARGET_VECTOR_ALIGNMENT): Define.
>> >
>> >         * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
>> >         vector type alignment instead of size.
>> >         * tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
>> >         element type size directly instead of computing it from alignment.
>> >         Fix variable naming and comment.
>> >
>> > testsuite/ChangeLog:
>> >
>> >         * lib/target-supports.exp
>> >         (check_effective_target_vect_natural_alignment): New function.
>> >         * gcc.dg/align-2.c: Only run on targets with natural alignment
>> >         of vector types.
>> >         * gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
>> >         alignment of vector types.
>
>
>> > OK for mainline?
>>
>> Ok.
>
> Would it be OK to backport this to 4.7 and possibly 4.6?
>
> This patch represents a change in the ABI on ARM (the alignment could
> potentially affect struct member offsets, for example), which could
> conceivably cause incompatibilities with code compiled with older
> versions of GCC.  We had originally decided we nevertheless want to
> implement this change on ARM, because:
>
> - GCC is now in compliance with the platform ABI document
> - the old code could actually be buggy since GCC *assumed* 16-byte
>   alignment that wasn't actually guaranteed
> - code actually affected by this change ought to be rare (code using
>   NEON vector types is rare in the first place, code using *structure
>   members* of such types is even rarer, and code using such structures
>   in cross-module APIs seems to be extremely rare)
>
> Nevertheless, given we do want to make this change, it would be best
> to roll it out as quickly as possible, to minimize the time period
> where people might use old (not yet fixed) compilers to generate
> non-ABI-compliant binaries.  Thus, we think it would be good for
> the change to be applied to all still open release branches as well.
>
> (Note that while the patch contains changes to common code, those
> should be no-ops for all targets that do not implement the new hook.)

I'll defer the decision to the target maintainers.  But please double-check
for any changes in the vectorizer parts when backporting to 4.6.

Thanks,
Richard.

> Bye,
> Ulrich
>
> --
>   Dr. Ulrich Weigand
>   GNU Toolchain for Linux on System z and Cell BE
>   Ulrich.Weigand@de.ibm.com
>

Re: [RFA 4.7/4.6] Re: [PATCH v2] Target-specific limits on vector alignment

[Richard Earnshaw]

On 07/08/12 16:04, Richard Guenther wrote:
> On Tue, Aug 7, 2012 at 4:56 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>> Richard Guenther wrote:
>>> On Fri, Jul 27, 2012 at 5:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>>>> ChangeLog:
>>>>
>>>>         * target.def (vector_alignment): New target hook.
>>>>         * doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
>>>>         * doc/tm.texi: Regenerate.
>>>>         * targhooks.c (default_vector_alignment): New function.
>>>>         * targhooks.h (default_vector_alignment): Add prototype.
>>>>         * stor-layout.c (layout_type): Use targetm.vector_alignment.
>>>>         * config/arm/arm.c (arm_vector_alignment): New function.
>>>>         (TARGET_VECTOR_ALIGNMENT): Define.
>>>>
>>>>         * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
>>>>         vector type alignment instead of size.
>>>>         * tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
>>>>         element type size directly instead of computing it from alignment.
>>>>         Fix variable naming and comment.
>>>>
>>>> testsuite/ChangeLog:
>>>>
>>>>         * lib/target-supports.exp
>>>>         (check_effective_target_vect_natural_alignment): New function.
>>>>         * gcc.dg/align-2.c: Only run on targets with natural alignment
>>>>         of vector types.
>>>>         * gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
>>>>         alignment of vector types.
>>
>>
>>>> OK for mainline?
>>>
>>> Ok.
>>
>> Would it be OK to backport this to 4.7 and possibly 4.6?
>>
>> This patch represents a change in the ABI on ARM (the alignment could
>> potentially affect struct member offsets, for example), which could
>> conceivably cause incompatibilities with code compiled with older
>> versions of GCC.  We had originally decided we nevertheless want to
>> implement this change on ARM, because:
>>
>> - GCC is now in compliance with the platform ABI document
>> - the old code could actually be buggy since GCC *assumed* 16-byte
>>   alignment that wasn't actually guaranteed
>> - code actually affected by this change ought to be rare (code using
>>   NEON vector types is rare in the first place, code using *structure
>>   members* of such types is even rarer, and code using such structures
>>   in cross-module APIs seems to be extremely rare)
>>
>> Nevertheless, given we do want to make this change, it would be best
>> to roll it out as quickly as possible, to minimize the time period
>> where people might use old (not yet fixed) compilers to generate
>> non-ABI-compliant binaries.  Thus, we think it would be good for
>> the change to be applied to all still open release branches as well.
>>
>> (Note that while the patch contains changes to common code, those
>> should be no-ops for all targets that do not implement the new hook.)
> 
> I'll defer the decision to the target maintainers.  But please double-check
> for any changes in the vectorizer parts when backporting to 4.6.
> 

My inclination is to back-port this to all maintained branches (for
consistency).

However, it does need to be release-noted.

R.

Re: [RFA 4.7/4.6] Re: [PATCH v2] Target-specific limits on vector alignment

[Ramana Radhakrishnan]

>> (Note that while the patch contains changes to common code, those
>> should be no-ops for all targets that do not implement the new hook.)
>
> I'll defer the decision to the target maintainers.


I'd rather have this consistent across all maintained release branches
today than to leave this for an ABI break in 4.8 timeframe.
 In addition I'd like this documented in changes.html for each of the
release branches.


Thanks,
Ramana

[RFA wwwdocs] Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Ramana Radhakrishnan wrote:

>  In addition I'd like this documented in changes.html for each of the
> release branches.

Richard Earnshaw wrote:

> However, it does need to be release-noted.

Would the following htdocs patch be OK with you?   Feel free to suggest
a more appropriate wording  ...

Bye,
Ulrich

Index: gcc-4.6/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.6/changes.html,v
retrieving revision 1.141
diff -u -p -r1.141 changes.html
--- gcc-4.6/changes.html	12 Jun 2012 10:43:41 -0000	1.141
+++ gcc-4.6/changes.html	10 Aug 2012 13:40:57 -0000
@@ -68,6 +68,12 @@
     default by <code>-Wall</code> flag and <code>-Wunused-but-set-parameter</code>
     by <code>-Wall -Wextra</code> flags.</li>
 
+    <li>In GCC version 4.6.4 the default alignment of vector types on ARM EABI
+    targets has been changed to 8 bytes (for types larger than 8 bytes in size),
+    to comply with the AAPCS.  This is an ABI change that affects e.g. layout of
+    structures having a member of vector type.  Code using such types may be
+    incompatible with binary objects built with older versions of GCC.</li>
+
     <li><p>Support for a number of older systems and recently
     unmaintained or untested target ports of GCC has been declared
     obsolete in GCC 4.6.  Unless there is activity to revive them, the
Index: gcc-4.7/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.123
diff -u -p -r1.123 changes.html
--- gcc-4.7/changes.html	23 Jul 2012 05:58:05 -0000	1.123
+++ gcc-4.7/changes.html	10 Aug 2012 13:40:57 -0000
@@ -121,6 +121,12 @@
     different GCC versions and with C++98/C++03 code compiled with any version.
     </li>
 
+    <li>In GCC version 4.7.2 the default alignment of vector types on ARM EABI
+    targets has been changed to 8 bytes (for types larger than 8 bytes in size),
+    to comply with the AAPCS.  This is an ABI change that affects e.g. layout of
+    structures having a member of vector type.  Code using such types may be
+    incompatible with binary objects built with older versions of GCC.</li>
+
     <li>More information on porting to GCC 4.7 from previous versions
     of GCC can be found in
     the <a href="http://gcc.gnu.org/gcc-4.7/porting_to.html">porting
Index: gcc-4.8/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v
retrieving revision 1.9
diff -u -p -r1.9 changes.html
--- gcc-4.8/changes.html	31 Jul 2012 12:13:01 -0000	1.9
+++ gcc-4.8/changes.html	10 Aug 2012 13:40:57 -0000
@@ -24,6 +24,12 @@ more information about requirements to b
 <p>The G++ namespace association extension, <code>__attribute ((strong))</code>,
 has been deprecated. Inline namespaces should be used instead.</p>
 
+<p>The default alignment of vector types on ARM EABI targets has been changed
+to 8 bytes (for types larger than 8 bytes in size), to comply with the AAPCS.
+This is an ABI change that affects e.g. layout of structures having a member
+of vector type.  Code using such types may be incompatible with binary objects
+built with older versions of GCC.</p>
+
 <h2>General Optimizer Improvements</h2>
 
   <ul>


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFA wwwdocs] Re: [PATCH v2] Target-specific limits on vector alignment

[Richard Earnshaw]

On 10/08/12 14:44, Ulrich Weigand wrote:
> Ramana Radhakrishnan wrote:
> 
>>  In addition I'd like this documented in changes.html for each of the
>> release branches.
> 
> Richard Earnshaw wrote:
> 
>> However, it does need to be release-noted.
> 
> Would the following htdocs patch be OK with you?   Feel free to suggest
> a more appropriate wording  ...
> 

I think we need to make it clear that this also fixes a bug in the
compiler that could lead to a run-time error.  Otherwise, people will be
asking why an abi-breaking change was made mid-cycle.

R.

> Bye,
> Ulrich
> 
> Index: gcc-4.6/changes.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.6/changes.html,v
> retrieving revision 1.141
> diff -u -p -r1.141 changes.html
> --- gcc-4.6/changes.html	12 Jun 2012 10:43:41 -0000	1.141
> +++ gcc-4.6/changes.html	10 Aug 2012 13:40:57 -0000
> @@ -68,6 +68,12 @@
>      default by <code>-Wall</code> flag and <code>-Wunused-but-set-parameter</code>
>      by <code>-Wall -Wextra</code> flags.</li>
>  
> +    <li>In GCC version 4.6.4 the default alignment of vector types on ARM EABI
> +    targets has been changed to 8 bytes (for types larger than 8 bytes in size),
> +    to comply with the AAPCS.  This is an ABI change that affects e.g. layout of
> +    structures having a member of vector type.  Code using such types may be
> +    incompatible with binary objects built with older versions of GCC.</li>
> +
>      <li><p>Support for a number of older systems and recently
>      unmaintained or untested target ports of GCC has been declared
>      obsolete in GCC 4.6.  Unless there is activity to revive them, the
> Index: gcc-4.7/changes.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
> retrieving revision 1.123
> diff -u -p -r1.123 changes.html
> --- gcc-4.7/changes.html	23 Jul 2012 05:58:05 -0000	1.123
> +++ gcc-4.7/changes.html	10 Aug 2012 13:40:57 -0000
> @@ -121,6 +121,12 @@
>      different GCC versions and with C++98/C++03 code compiled with any version.
>      </li>
>  
> +    <li>In GCC version 4.7.2 the default alignment of vector types on ARM EABI
> +    targets has been changed to 8 bytes (for types larger than 8 bytes in size),
> +    to comply with the AAPCS.  This is an ABI change that affects e.g. layout of
> +    structures having a member of vector type.  Code using such types may be
> +    incompatible with binary objects built with older versions of GCC.</li>
> +
>      <li>More information on porting to GCC 4.7 from previous versions
>      of GCC can be found in
>      the <a href="http://gcc.gnu.org/gcc-4.7/porting_to.html">porting
> Index: gcc-4.8/changes.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v
> retrieving revision 1.9
> diff -u -p -r1.9 changes.html
> --- gcc-4.8/changes.html	31 Jul 2012 12:13:01 -0000	1.9
> +++ gcc-4.8/changes.html	10 Aug 2012 13:40:57 -0000
> @@ -24,6 +24,12 @@ more information about requirements to b
>  <p>The G++ namespace association extension, <code>__attribute ((strong))</code>,
>  has been deprecated. Inline namespaces should be used instead.</p>
>  
> +<p>The default alignment of vector types on ARM EABI targets has been changed
> +to 8 bytes (for types larger than 8 bytes in size), to comply with the AAPCS.
> +This is an ABI change that affects e.g. layout of structures having a member
> +of vector type.  Code using such types may be incompatible with binary objects
> +built with older versions of GCC.</p>
> +
>  <h2>General Optimizer Improvements</h2>
>  
>    <ul>
> 
> 

Re: [RFA wwwdocs] Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Earnshaw wrote:
> On 10/08/12 14:44, Ulrich Weigand wrote:
> > Would the following htdocs patch be OK with you?   Feel free to suggest
> > a more appropriate wording  ...
> 
> I think we need to make it clear that this also fixes a bug in the
> compiler that could lead to a run-time error.  Otherwise, people will be
> asking why an abi-breaking change was made mid-cycle.

Something along the following lines?

<p>The default alignment of vector types on ARM EABI targets has been changed
to 8 bytes (for types larger than 8 bytes in size).  This change was necessary
in order to fix a bug that could lead to generation of wrong code in certain
cases.  It also brings the ABI as implemented by GCC back in compliance with
the AAPCS.  This is an ABI change that affects e.g. layout of structures having
a member of vector type.  Code using such types may be incompatible with binary
objects built with older versions of GCC.</p>

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFA wwwdocs] Re: [PATCH v2] Target-specific limits on vector alignment

[Richard Earnshaw]

On 10/08/12 16:18, Ulrich Weigand wrote:
> Richard Earnshaw wrote:
>> On 10/08/12 14:44, Ulrich Weigand wrote:
>>> Would the following htdocs patch be OK with you?   Feel free to suggest
>>> a more appropriate wording  ...
>>
>> I think we need to make it clear that this also fixes a bug in the
>> compiler that could lead to a run-time error.  Otherwise, people will be
>> asking why an abi-breaking change was made mid-cycle.
> 
> Something along the following lines?
> 
> <p>The default alignment of vector types on ARM EABI targets has been changed
> to 8 bytes (for types larger than 8 bytes in size).  This change was necessary
> in order to fix a bug that could lead to generation of wrong code in certain
> cases.  It also brings the ABI as implemented by GCC back in compliance with
> the AAPCS.  This is an ABI change that affects e.g. layout of structures having
> a member of vector type.  Code using such types may be incompatible with binary
> objects built with older versions of GCC.</p>
> 
> Bye,
> Ulrich
> 

How about:

<p>On ARM, a bug has been fixed in GCC's implementation of the AAPCS
rules for the layout of vectors that could lead to wrong code being
generated.  Vectors larger than 8 bytes in size are now by default
aligned to an 8-byte boundary.  This is an ABI change: code that makes
explicit use of vector types may be incompatible with binary objects
built with older versions of GCC.  Auto-vectorized code is not affected
by this change.</p>

R.

Re: [RFA wwwdocs] Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Earnshaw wrote:

> How about:
> 
> <p>On ARM, a bug has been fixed in GCC's implementation of the AAPCS
> rules for the layout of vectors that could lead to wrong code being
> generated.  Vectors larger than 8 bytes in size are now by default
> aligned to an 8-byte boundary.  This is an ABI change: code that makes
> explicit use of vector types may be incompatible with binary objects
> built with older versions of GCC.  Auto-vectorized code is not affected
> by this change.</p>

Sounds better to me, thanks :-)

Here's what I have committed to wwwdocs.

Bye,
Ulrich

Index: gcc-4.6/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.6/changes.html,v
retrieving revision 1.141
diff -c -p -r1.141 changes.html
*** gcc-4.6/changes.html	12 Jun 2012 10:43:41 -0000	1.141
--- gcc-4.6/changes.html	10 Aug 2012 16:23:57 -0000
***************
*** 68,73 ****
--- 68,82 ----
      default by <code>-Wall</code> flag and <code>-Wunused-but-set-parameter</code>
      by <code>-Wall -Wextra</code> flags.</li>
  
+     <li>On ARM, a bug has been fixed in GCC's implementation of the AAPCS
+     rules for the layout of vectors that could lead to wrong code being
+     generated.  Vectors larger than 8 bytes in size are now by default
+     aligned to an 8-byte boundary.  This is an ABI change: code that makes
+     explicit use of vector types may be incompatible with binary objects
+     built with older versions of GCC.  Auto-vectorized code is not affected
+     by this change.  (This change affects GCC versions 4.6.4 and later,
+     with the exception of versions 4.7.0 and 4.7.1.)</li>
+ 
      <li><p>Support for a number of older systems and recently
      unmaintained or untested target ports of GCC has been declared
      obsolete in GCC 4.6.  Unless there is activity to revive them, the
Index: gcc-4.7/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.123
diff -c -p -r1.123 changes.html
*** gcc-4.7/changes.html	23 Jul 2012 05:58:05 -0000	1.123
--- gcc-4.7/changes.html	10 Aug 2012 16:23:58 -0000
***************
*** 121,126 ****
--- 121,134 ----
      different GCC versions and with C++98/C++03 code compiled with any version.
      </li>
  
+     <li>On ARM, a bug has been fixed in GCC's implementation of the AAPCS
+     rules for the layout of vectors that could lead to wrong code being
+     generated.  Vectors larger than 8 bytes in size are now by default
+     aligned to an 8-byte boundary.  This is an ABI change: code that makes
+     explicit use of vector types may be incompatible with binary objects
+     built with older versions of GCC.  Auto-vectorized code is not affected
+     by this change.  (This change affects GCC versions 4.7.2 and later.)</li>
+ 
      <li>More information on porting to GCC 4.7 from previous versions
      of GCC can be found in
      the <a href="http://gcc.gnu.org/gcc-4.7/porting_to.html">porting
Index: gcc-4.8/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v
retrieving revision 1.9
diff -c -p -r1.9 changes.html
*** gcc-4.8/changes.html	31 Jul 2012 12:13:01 -0000	1.9
--- gcc-4.8/changes.html	10 Aug 2012 16:23:58 -0000
*************** more information about requirements to b
*** 24,29 ****
--- 24,37 ----
  <p>The G++ namespace association extension, <code>__attribute ((strong))</code>,
  has been deprecated. Inline namespaces should be used instead.</p>
  
+ <p>On ARM, a bug has been fixed in GCC's implementation of the AAPCS
+ rules for the layout of vectors that could lead to wrong code being
+ generated.  Vectors larger than 8 bytes in size are now by default
+ aligned to an 8-byte boundary.  This is an ABI change: code that makes
+ explicit use of vector types may be incompatible with binary objects
+ built with older versions of GCC.  Auto-vectorized code is not affected
+ by this change.</p>
+ 
  <h2>General Optimizer Improvements</h2>
  
    <ul>

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[commit 4.7] Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Guenther wrote:
> On Tue, Aug 7, 2012 at 4:56 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > Would it be OK to backport this to 4.7 and possibly 4.6?

> I'll defer the decision to the target maintainers.  But please double-check
> for any changes in the vectorizer parts when backporting to 4.6.

Thanks!  For reference, there's the version as committed to 4.7.

Bye,
Ulrich

ChangeLog:

	Backport from mainline:

        2012-07-30  Ulrich Weigand  <ulrich.weigand@linaro.org>
                    Richard Earnshaw  <rearnsha@arm.com>

	* target.def (vector_alignment): New target hook.
	* doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_vector_alignment): New function.
	* targhooks.h (default_vector_alignment): Add prototype.
	* stor-layout.c (layout_type): Use targetm.vector_alignment.
	* config/arm/arm.c (arm_vector_alignment): New function.
	(TARGET_VECTOR_ALIGNMENT): Define.

	* tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
	vector type alignment instead of size.
	* tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
	element type size directly instead of computing it from alignment.
	Fix variable naming and comment.


testsuite/ChangeLog:

	Backport from mainline:

        2012-07-30  Ulrich Weigand  <ulrich.weigand@linaro.org>
                    Richard Earnshaw  <rearnsha@arm.com>

	* lib/target-supports.exp
	(check_effective_target_vect_natural_alignment): New function.
	* gcc.dg/align-2.c: Only run on targets with natural alignment
	of vector types.
	* gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
	alignment of vector types.


Index: gcc/doc/tm.texi
===================================================================
*** gcc/doc/tm.texi	(revision 190202)
--- gcc/doc/tm.texi	(working copy)
*************** make it all fit in fewer cache lines.
*** 1105,1110 ****
--- 1105,1118 ----
  If the value of this macro has a type, it should be an unsigned type.
  @end defmac
  
+ @deftypefn {Target Hook} HOST_WIDE_INT TARGET_VECTOR_ALIGNMENT (const_tree @var{type})
+ This hook can be used to define the alignment for a vector of type
+ @var{type}, in order to comply with a platform ABI.  The default is to
+ require natural alignment for vector types.  The alignment returned by
+ this hook must be a power-of-two multiple of the default alignment of
+ the vector element type.
+ @end deftypefn
+ 
  @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
  If defined, a C expression to compute the alignment for stack slot.
  @var{type} is the data type, @var{mode} is the widest mode available,
Index: gcc/doc/tm.texi.in
===================================================================
*** gcc/doc/tm.texi.in	(revision 190202)
--- gcc/doc/tm.texi.in	(working copy)
*************** make it all fit in fewer cache lines.
*** 1093,1098 ****
--- 1093,1100 ----
  If the value of this macro has a type, it should be an unsigned type.
  @end defmac
  
+ @hook TARGET_VECTOR_ALIGNMENT
+ 
  @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
  If defined, a C expression to compute the alignment for stack slot.
  @var{type} is the data type, @var{mode} is the widest mode available,
Index: gcc/targhooks.c
===================================================================
*** gcc/targhooks.c	(revision 190202)
--- gcc/targhooks.c	(working copy)
*************** tree default_mangle_decl_assembler_name 
*** 939,944 ****
--- 939,951 ----
     return id;
  }
  
+ /* Default to natural alignment for vector types.  */
+ HOST_WIDE_INT
+ default_vector_alignment (const_tree type)
+ {
+   return tree_low_cst (TYPE_SIZE (type), 0);
+ }
+ 
  bool
  default_builtin_vector_alignment_reachable (const_tree type, bool is_packed)
  {
Index: gcc/targhooks.h
===================================================================
*** gcc/targhooks.h	(revision 190202)
--- gcc/targhooks.h	(working copy)
*************** extern int default_builtin_vectorization
*** 83,88 ****
--- 83,90 ----
  
  extern tree default_builtin_reciprocal (unsigned int, bool, bool);
  
+ extern HOST_WIDE_INT default_vector_alignment (const_tree);
+ 
  extern bool default_builtin_vector_alignment_reachable (const_tree, bool);
  extern bool
  default_builtin_support_vector_misalignment (enum machine_mode mode,
Index: gcc/target.def
===================================================================
*** gcc/target.def	(revision 190202)
--- gcc/target.def	(working copy)
*************** DEFHOOK
*** 1615,1620 ****
--- 1615,1630 ----
   bool, (enum machine_mode mode),
   hook_bool_mode_false)
  
+ DEFHOOK
+ (vector_alignment,
+  "This hook can be used to define the alignment for a vector of type\n\
+ @var{type}, in order to comply with a platform ABI.  The default is to\n\
+ require natural alignment for vector types.  The alignment returned by\n\
+ this hook must be a power-of-two multiple of the default alignment of\n\
+ the vector element type.",
+  HOST_WIDE_INT, (const_tree type),
+  default_vector_alignment)
+ 
  /* True if we should try to use a scalar mode to represent an array,
     overriding the usual MAX_FIXED_MODE limit.  */
  DEFHOOK
Index: gcc/tree-vect-loop-manip.c
===================================================================
*** gcc/tree-vect-loop-manip.c	(revision 190202)
--- gcc/tree-vect-loop-manip.c	(working copy)
*************** vect_do_peeling_for_loop_bound (loop_vec
*** 1993,1999 ****
     If the misalignment of DR is known at compile time:
       addr_mis = int mis = DR_MISALIGNMENT (dr);
     Else, compute address misalignment in bytes:
!      addr_mis = addr & (vectype_size - 1)
  
     prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
  
--- 1993,1999 ----
     If the misalignment of DR is known at compile time:
       addr_mis = int mis = DR_MISALIGNMENT (dr);
     Else, compute address misalignment in bytes:
!      addr_mis = addr & (vectype_align - 1)
  
     prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
  
*************** vect_gen_niters_for_prolog_loop (loop_ve
*** 2049,2057 ****
        tree ptr_type = TREE_TYPE (start_addr);
        tree size = TYPE_SIZE (ptr_type);
        tree type = lang_hooks.types.type_for_size (tree_low_cst (size, 1), 1);
!       tree vectype_size_minus_1 = build_int_cst (type, vectype_align - 1);
!       tree elem_size_log =
!         build_int_cst (type, exact_log2 (vectype_align/nelements));
        tree nelements_minus_1 = build_int_cst (type, nelements - 1);
        tree nelements_tree = build_int_cst (type, nelements);
        tree byte_misalign;
--- 2049,2058 ----
        tree ptr_type = TREE_TYPE (start_addr);
        tree size = TYPE_SIZE (ptr_type);
        tree type = lang_hooks.types.type_for_size (tree_low_cst (size, 1), 1);
!       tree vectype_align_minus_1 = build_int_cst (type, vectype_align - 1);
!       HOST_WIDE_INT elem_size =
! 		int_cst_value (TYPE_SIZE_UNIT (TREE_TYPE (vectype)));
!       tree elem_size_log = build_int_cst (type, exact_log2 (elem_size));
        tree nelements_minus_1 = build_int_cst (type, nelements - 1);
        tree nelements_tree = build_int_cst (type, nelements);
        tree byte_misalign;
*************** vect_gen_niters_for_prolog_loop (loop_ve
*** 2060,2069 ****
        new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
        gcc_assert (!new_bb);
  
!       /* Create:  byte_misalign = addr & (vectype_size - 1)  */
        byte_misalign =
          fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr), 
!                      vectype_size_minus_1);
  
        /* Create:  elem_misalign = byte_misalign / element_size  */
        elem_misalign =
--- 2061,2070 ----
        new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
        gcc_assert (!new_bb);
  
!       /* Create:  byte_misalign = addr & (vectype_align - 1)  */
        byte_misalign =
          fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr), 
!                      vectype_align_minus_1);
  
        /* Create:  elem_misalign = byte_misalign / element_size  */
        elem_misalign =
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
*** gcc/testsuite/lib/target-supports.exp	(revision 190202)
--- gcc/testsuite/lib/target-supports.exp	(working copy)
*************** proc check_effective_target_natural_alig
*** 3370,3375 ****
--- 3370,3395 ----
      return $et_natural_alignment_64_saved
  }
  
+ # Return 1 if all vector types are naturally aligned (aligned to their
+ # type-size), 0 otherwise.
+ #
+ # This won't change for different subtargets so cache the result.
+ 
+ proc check_effective_target_vect_natural_alignment { } {
+     global et_vect_natural_alignment
+ 
+     if [info exists et_vect_natural_alignment_saved] {
+         verbose "check_effective_target_vect_natural_alignment: using cached result" 2
+     } else {
+         set et_vect_natural_alignment_saved 1
+         if { [check_effective_target_arm_eabi] } {
+             set et_vect_natural_alignment_saved 0
+         }
+     }
+     verbose "check_effective_target_vect_natural_alignment: returning $et_vect_natural_alignment_saved" 2
+     return $et_vect_natural_alignment_saved
+ }
+ 
  # Return 1 if vector alignment (for types of size 32 bit or less) is reachable, 0 otherwise.
  #
  # This won't change for different subtargets so cache the result.
Index: gcc/testsuite/gcc.dg/align-2.c
===================================================================
*** gcc/testsuite/gcc.dg/align-2.c	(revision 190202)
--- gcc/testsuite/gcc.dg/align-2.c	(working copy)
***************
*** 1,5 ****
  /* PR 17962 */
! /* { dg-do compile } */
  /* { dg-options "" } */
  
  typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
--- 1,5 ----
  /* PR 17962 */
! /* { dg-do compile { target vect_natural_alignment } } */
  /* { dg-options "" } */
  
  typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
Index: gcc/testsuite/gcc.dg/vect/slp-25.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/slp-25.c	(revision 190202)
--- gcc/testsuite/gcc.dg/vect/slp-25.c	(working copy)
*************** int main (void)
*** 56,60 ****
  
  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 56,60 ----
  
  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align || { ! vect_natural_alignment } } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/stor-layout.c
===================================================================
*** gcc/stor-layout.c	(revision 190202)
--- gcc/stor-layout.c	(working copy)
*************** layout_type (tree type)
*** 2108,2116 ****
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
  					    bitsize_int (nunits));
  
! 	/* Always naturally align vectors.  This prevents ABI changes
! 	   depending on whether or not native vector modes are supported.  */
! 	TYPE_ALIGN (type) = tree_low_cst (TYPE_SIZE (type), 0);
          break;
        }
  
--- 2108,2124 ----
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
  					    bitsize_int (nunits));
  
! 	/* For vector types, we do not default to the mode's alignment.
! 	   Instead, query a target hook, defaulting to natural alignment.
! 	   This prevents ABI changes depending on whether or not native
! 	   vector modes are supported.  */
! 	TYPE_ALIGN (type) = targetm.vector_alignment (type);
! 
! 	/* However, if the underlying mode requires a bigger alignment than
! 	   what the target hook provides, we cannot use the mode.  For now,
! 	   simply reject that case.  */
! 	gcc_assert (TYPE_ALIGN (type)
! 		    >= GET_MODE_ALIGNMENT (TYPE_MODE (type)));
          break;
        }
  
Index: gcc/tree-vect-data-refs.c
===================================================================
*** gcc/tree-vect-data-refs.c	(revision 190202)
--- gcc/tree-vect-data-refs.c	(working copy)
*************** vect_update_misalignment_for_peel (struc
*** 1023,1029 ****
        int misal = DR_MISALIGNMENT (dr);
        tree vectype = STMT_VINFO_VECTYPE (stmt_info);
        misal += negative ? -npeel * dr_size : npeel * dr_size;
!       misal &= GET_MODE_SIZE (TYPE_MODE (vectype)) - 1;
        SET_DR_MISALIGNMENT (dr, misal);
        return;
      }
--- 1023,1029 ----
        int misal = DR_MISALIGNMENT (dr);
        tree vectype = STMT_VINFO_VECTYPE (stmt_info);
        misal += negative ? -npeel * dr_size : npeel * dr_size;
!       misal &= (TYPE_ALIGN (vectype) / BITS_PER_UNIT) - 1;
        SET_DR_MISALIGNMENT (dr, misal);
        return;
      }
Index: gcc/config/arm/arm.c
===================================================================
*** gcc/config/arm/arm.c	(revision 190202)
--- gcc/config/arm/arm.c	(working copy)
*************** static bool arm_array_mode_supported_p (
*** 258,263 ****
--- 258,264 ----
  					unsigned HOST_WIDE_INT);
  static enum machine_mode arm_preferred_simd_mode (enum machine_mode);
  static bool arm_class_likely_spilled_p (reg_class_t);
+ static HOST_WIDE_INT arm_vector_alignment (const_tree type);
  static bool arm_vector_alignment_reachable (const_tree type, bool is_packed);
  static bool arm_builtin_support_vector_misalignment (enum machine_mode mode,
  						     const_tree type,
*************** static const struct attribute_spec arm_a
*** 603,608 ****
--- 604,612 ----
  #undef TARGET_CLASS_LIKELY_SPILLED_P
  #define TARGET_CLASS_LIKELY_SPILLED_P arm_class_likely_spilled_p
  
+ #undef TARGET_VECTOR_ALIGNMENT
+ #define TARGET_VECTOR_ALIGNMENT arm_vector_alignment
+ 
  #undef TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE
  #define TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE \
    arm_vector_alignment_reachable
*************** arm_have_conditional_execution (void)
*** 24482,24487 ****
--- 24486,24503 ----
    return !TARGET_THUMB1;
  }
  
+ /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
+ static HOST_WIDE_INT
+ arm_vector_alignment (const_tree type)
+ {
+   HOST_WIDE_INT align = tree_low_cst (TYPE_SIZE (type), 0);
+ 
+   if (TARGET_AAPCS_BASED)
+     align = MIN (align, 64);
+ 
+   return align;
+ }
+ 
  static unsigned int
  arm_autovectorize_vector_sizes (void)
  {

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[commit 4.6] Re: [PATCH v2] Target-specific limits on vector alignment

[Ulrich Weigand]

Richard Guenther wrote:
> On Tue, Aug 7, 2012 at 4:56 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > Would it be OK to backport this to 4.7 and possibly 4.6?
 
> I'll defer the decision to the target maintainers.  But please double-check
> for any changes in the vectorizer parts when backporting to 4.6.

And here the change as committed to 4.6.  I didn't find any relevant changes
in vectorizer code; however, I had to back-port a couple of testcase changes
(to the vect-peel-* tests) to avoid seeing regressions there.

Bye,
Ulrich

ChangeLog:

	Backport from mainline
	2012-07-30  Ulrich Weigand  <ulrich.weigand@linaro.org>
		    Richard Earnshaw  <rearnsha@arm.com>

	* target.def (vector_alignment): New target hook.
	* doc/tm.texi.in (TARGET_VECTOR_ALIGNMENT): Document new hook.
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_vector_alignment): New function.
	* targhooks.h (default_vector_alignment): Add prototype.
	* stor-layout.c (layout_type): Use targetm.vector_alignment.
	* config/arm/arm.c (arm_vector_alignment): New function.
	(TARGET_VECTOR_ALIGNMENT): Define.

	* tree-vect-data-refs.c (vect_update_misalignment_for_peel): Use
	vector type alignment instead of size.
	* tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Use
	element type size directly instead of computing it from alignment.
	Fix variable naming and comment.


testsuite/ChangeLog:

	Backport from mainline
	2012-07-30  Ulrich Weigand  <ulrich.weigand@linaro.org>

	* lib/target-supports.exp
	(check_effective_target_vect_natural_alignment): New function.
	* gcc.dg/align-2.c: Only run on targets with natural alignment
	of vector types.
	* gcc.dg/vect/slp-25.c: Adjust tests for targets without natural
	alignment of vector types.

	2011-12-21  Michael Zolotukhin  <michael.v.zolotukhin@intel.com>

        * gcc.dg/vect/vect-peel-1.c: Adjust test diag-scans to fix fail on AVX.
        * gcc.dg/vect/vect-peel-2.c: Ditto.

	2011-06-21  Ira Rosen  <ira.rosen@linaro.org>

        PR testsuite/49443
        * gcc.dg/vect/vect-peel-3.c: Expect to fail on vect_no_align
        targets.
        * gcc.dg/vect/vect-peel-4.c: Likewise.

	2011-06-14  Ira Rosen  <ira.rosen@linaro.org>

        * gcc.dg/vect/vect-peel-3.c: Adjust misalignment values
        for double-word vectors.
        * gcc.dg/vect/vect-peel-4.c: Likewise.


Index: gcc/doc/tm.texi
===================================================================
*** gcc/doc/tm.texi	(revision 190202)
--- gcc/doc/tm.texi	(working copy)
*************** make it all fit in fewer cache lines.
*** 1118,1123 ****
--- 1118,1131 ----
  If the value of this macro has a type, it should be an unsigned type.
  @end defmac
  
+ @deftypefn {Target Hook} HOST_WIDE_INT TARGET_VECTOR_ALIGNMENT (const_tree @var{type})
+ This hook can be used to define the alignment for a vector of type
+ @var{type}, in order to comply with a platform ABI.  The default is to
+ require natural alignment for vector types.  The alignment returned by
+ this hook must be a power-of-two multiple of the default alignment of
+ the vector element type.
+ @end deftypefn
+ 
  @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
  If defined, a C expression to compute the alignment for stack slot.
  @var{type} is the data type, @var{mode} is the widest mode available,
Index: gcc/doc/tm.texi.in
===================================================================
*** gcc/doc/tm.texi.in	(revision 190202)
--- gcc/doc/tm.texi.in	(working copy)
*************** make it all fit in fewer cache lines.
*** 1108,1113 ****
--- 1108,1115 ----
  If the value of this macro has a type, it should be an unsigned type.
  @end defmac
  
+ @hook TARGET_VECTOR_ALIGNMENT
+ 
  @defmac STACK_SLOT_ALIGNMENT (@var{type}, @var{mode}, @var{basic-align})
  If defined, a C expression to compute the alignment for stack slot.
  @var{type} is the data type, @var{mode} is the widest mode available,
Index: gcc/targhooks.c
===================================================================
*** gcc/targhooks.c	(revision 190202)
--- gcc/targhooks.c	(working copy)
*************** tree default_mangle_decl_assembler_name 
*** 979,984 ****
--- 979,991 ----
     return id;
  }
  
+ /* Default to natural alignment for vector types.  */
+ HOST_WIDE_INT
+ default_vector_alignment (const_tree type)
+ {
+   return tree_low_cst (TYPE_SIZE (type), 0);
+ }
+ 
  bool
  default_builtin_vector_alignment_reachable (const_tree type, bool is_packed)
  {
Index: gcc/targhooks.h
===================================================================
*** gcc/targhooks.h	(revision 190202)
--- gcc/targhooks.h	(working copy)
*************** extern int default_builtin_vectorization
*** 85,90 ****
--- 85,92 ----
  
  extern tree default_builtin_reciprocal (unsigned int, bool, bool);
  
+ extern HOST_WIDE_INT default_vector_alignment (const_tree);
+ 
  extern bool default_builtin_vector_alignment_reachable (const_tree, bool);
  extern bool
  default_builtin_support_vector_misalignment (enum machine_mode mode,
Index: gcc/target.def
===================================================================
*** gcc/target.def	(revision 190202)
--- gcc/target.def	(working copy)
*************** DEFHOOK
*** 1611,1616 ****
--- 1611,1626 ----
   bool, (enum machine_mode mode),
   hook_bool_mode_false)
  
+ DEFHOOK
+ (vector_alignment,
+  "This hook can be used to define the alignment for a vector of type\n\
+ @var{type}, in order to comply with a platform ABI.  The default is to\n\
+ require natural alignment for vector types.  The alignment returned by\n\
+ this hook must be a power-of-two multiple of the default alignment of\n\
+ the vector element type.",
+  HOST_WIDE_INT, (const_tree type),
+  default_vector_alignment)
+ 
  /* Compute cost of moving data from a register of class FROM to one of
     TO, using MODE.  */
  DEFHOOK
Index: gcc/tree-vect-loop-manip.c
===================================================================
*** gcc/tree-vect-loop-manip.c	(revision 190202)
--- gcc/tree-vect-loop-manip.c	(working copy)
*************** vect_do_peeling_for_loop_bound (loop_vec
*** 2008,2014 ****
     If the misalignment of DR is known at compile time:
       addr_mis = int mis = DR_MISALIGNMENT (dr);
     Else, compute address misalignment in bytes:
!      addr_mis = addr & (vectype_size - 1)
  
     prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
  
--- 2008,2014 ----
     If the misalignment of DR is known at compile time:
       addr_mis = int mis = DR_MISALIGNMENT (dr);
     Else, compute address misalignment in bytes:
!      addr_mis = addr & (vectype_align - 1)
  
     prolog_niters = min (LOOP_NITERS, ((VF - addr_mis/elem_size)&(VF-1))/step)
  
*************** vect_gen_niters_for_prolog_loop (loop_ve
*** 2065,2073 ****
        tree ptr_type = TREE_TYPE (start_addr);
        tree size = TYPE_SIZE (ptr_type);
        tree type = lang_hooks.types.type_for_size (tree_low_cst (size, 1), 1);
!       tree vectype_size_minus_1 = build_int_cst (type, vectype_align - 1);
!       tree elem_size_log =
!         build_int_cst (type, exact_log2 (vectype_align/nelements));
        tree nelements_minus_1 = build_int_cst (type, nelements - 1);
        tree nelements_tree = build_int_cst (type, nelements);
        tree byte_misalign;
--- 2065,2074 ----
        tree ptr_type = TREE_TYPE (start_addr);
        tree size = TYPE_SIZE (ptr_type);
        tree type = lang_hooks.types.type_for_size (tree_low_cst (size, 1), 1);
!       tree vectype_align_minus_1 = build_int_cst (type, vectype_align - 1);
!       HOST_WIDE_INT elem_size =
! 		int_cst_value (TYPE_SIZE_UNIT (TREE_TYPE (vectype)));
!       tree elem_size_log = build_int_cst (type, exact_log2 (elem_size));
        tree nelements_minus_1 = build_int_cst (type, nelements - 1);
        tree nelements_tree = build_int_cst (type, nelements);
        tree byte_misalign;
*************** vect_gen_niters_for_prolog_loop (loop_ve
*** 2076,2085 ****
        new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
        gcc_assert (!new_bb);
  
!       /* Create:  byte_misalign = addr & (vectype_size - 1)  */
        byte_misalign =
          fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr), 
!                      vectype_size_minus_1);
  
        /* Create:  elem_misalign = byte_misalign / element_size  */
        elem_misalign =
--- 2077,2086 ----
        new_bb = gsi_insert_seq_on_edge_immediate (pe, new_stmts);
        gcc_assert (!new_bb);
  
!       /* Create:  byte_misalign = addr & (vectype_align - 1)  */
        byte_misalign =
          fold_build2 (BIT_AND_EXPR, type, fold_convert (type, start_addr), 
!                      vectype_align_minus_1);
  
        /* Create:  elem_misalign = byte_misalign / element_size  */
        elem_misalign =
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
*** gcc/testsuite/lib/target-supports.exp	(revision 190202)
--- gcc/testsuite/lib/target-supports.exp	(working copy)
*************** proc check_effective_target_natural_alig
*** 2976,2981 ****
--- 2976,3001 ----
      return $et_natural_alignment_64_saved
  }
  
+ # Return 1 if all vector types are naturally aligned (aligned to their
+ # type-size), 0 otherwise.
+ #
+ # This won't change for different subtargets so cache the result.
+ 
+ proc check_effective_target_vect_natural_alignment { } {
+     global et_vect_natural_alignment
+ 
+     if [info exists et_vect_natural_alignment_saved] {
+         verbose "check_effective_target_vect_natural_alignment: using cached result" 2
+     } else {
+         set et_vect_natural_alignment_saved 1
+         if { [check_effective_target_arm_eabi] } {
+             set et_vect_natural_alignment_saved 0
+         }
+     }
+     verbose "check_effective_target_vect_natural_alignment: returning $et_vect_natural_alignment_saved" 2
+     return $et_vect_natural_alignment_saved
+ }
+ 
  # Return 1 if vector alignment (for types of size 32 bit or less) is reachable, 0 otherwise.
  #
  # This won't change for different subtargets so cache the result.
Index: gcc/testsuite/gcc.dg/align-2.c
===================================================================
*** gcc/testsuite/gcc.dg/align-2.c	(revision 190202)
--- gcc/testsuite/gcc.dg/align-2.c	(working copy)
***************
*** 1,5 ****
  /* PR 17962 */
! /* { dg-do compile } */
  /* { dg-options "" } */
  
  typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
--- 1,5 ----
  /* PR 17962 */
! /* { dg-do compile { target vect_natural_alignment } } */
  /* { dg-options "" } */
  
  typedef float v4 __attribute__((vector_size(sizeof(float)*4)));
Index: gcc/testsuite/gcc.dg/vect/vect-peel-1.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/vect-peel-1.c	(revision 190202)
--- gcc/testsuite/gcc.dg/vect/vect-peel-1.c	(working copy)
*************** int main (void)
*** 49,54 ****
  }
  
  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect" { target vect_element_align } } } */
  /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 49,54 ----
  }
  
  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect" { target { { vect_element_align } && { vect_aligned_arrays } } } } } */
  /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/testsuite/gcc.dg/vect/vect-peel-2.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/vect-peel-2.c	(revision 190202)
--- gcc/testsuite/gcc.dg/vect/vect-peel-2.c	(working copy)
*************** int main (void)
*** 50,55 ****
  }
  
  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect" { target vect_element_align } } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" { target vect_element_align } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 50,55 ----
  }
  
  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect" { target { { vect_element_align } && { vect_aligned_arrays } } } } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" { target { { vect_element_align } && { vect_aligned_arrays } } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/testsuite/gcc.dg/vect/vect-peel-3.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/vect-peel-3.c	(revision 190202)
--- gcc/testsuite/gcc.dg/vect/vect-peel-3.c	(working copy)
***************
*** 4,12 ****
  #include "tree-vect.h"
  
  #define N 128
! #define RES 21888 
! 
! /* unaligned store.  */
  
  int ib[N+10];
  int ia[N+10];
--- 4,10 ----
  #include "tree-vect.h"
  
  #define N 128
! #define RES 21640 
  
  int ib[N+10];
  int ia[N+10];
*************** int main1 ()
*** 18,28 ****
    int i, suma = 0, sumb = 0, sumc = 0;
  
    /* ib and ic have same misalignment, we peel to align them.  */
!   for (i = 1; i <= N; i++)
      {
        suma += ia[i];
!       sumb += ib[i+6];
!       sumc += ic[i+2];
      }
  
    /* check results:  */
--- 16,26 ----
    int i, suma = 0, sumb = 0, sumc = 0;
  
    /* ib and ic have same misalignment, we peel to align them.  */
!   for (i = 0; i <= N; i++)
      {
        suma += ia[i];
!       sumb += ib[i+5];
!       sumc += ic[i+1];
      }
  
    /* check results:  */
*************** int main (void)
*** 49,55 ****
    return main1 ();
  }
  
! /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect"  { xfail vect_no_align } } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 47,53 ----
    return main1 ();
  }
  
! /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { xfail vect_no_align } } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect"  { xfail vect_no_align } } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" { xfail vect_no_align } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/testsuite/gcc.dg/vect/vect-peel-4.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/vect-peel-4.c	(revision 190202)
--- gcc/testsuite/gcc.dg/vect/vect-peel-4.c	(working copy)
*************** int main1 ()
*** 16,28 ****
    /* Don't peel keeping one load and the store aligned.  */
    for (i = 0; i <= N; i++)
      {
!       ia[i] = ib[i] + ib[i+6];
      }
  
    /* check results:  */
    for (i = 1; i <= N; i++)
      {
!       if (ia[i] != ib[i] + ib[i+6])
          abort ();
      }
  
--- 16,28 ----
    /* Don't peel keeping one load and the store aligned.  */
    for (i = 0; i <= N; i++)
      {
!       ia[i] = ib[i] + ib[i+5];
      }
  
    /* check results:  */
    for (i = 1; i <= N; i++)
      {
!       if (ia[i] != ib[i] + ib[i+5])
          abort ();
      }
  
*************** int main (void)
*** 44,50 ****
    return main1 ();
  }
  
! /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect"  { xfail vect_no_align } } } */
  /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 0 "vect" } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 44,50 ----
    return main1 ();
  }
  
! /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { xfail vect_no_align } } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 1 "vect"  { xfail vect_no_align } } } */
  /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 0 "vect" } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/testsuite/gcc.dg/vect/slp-25.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/slp-25.c	(revision 190202)
--- gcc/testsuite/gcc.dg/vect/slp-25.c	(working copy)
*************** int main (void)
*** 57,61 ****
  
  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
--- 57,61 ----
  
  /* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  } } */
  /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
! /* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { vect_no_align || { ! vect_natural_alignment } } } } } */
  /* { dg-final { cleanup-tree-dump "vect" } } */
Index: gcc/stor-layout.c
===================================================================
*** gcc/stor-layout.c	(revision 190202)
--- gcc/stor-layout.c	(working copy)
*************** layout_type (tree type)
*** 1927,1935 ****
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
  					    bitsize_int (nunits), 0);
  
! 	/* Always naturally align vectors.  This prevents ABI changes
! 	   depending on whether or not native vector modes are supported.  */
! 	TYPE_ALIGN (type) = tree_low_cst (TYPE_SIZE (type), 0);
          break;
        }
  
--- 1927,1943 ----
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
  					    bitsize_int (nunits), 0);
  
! 	/* For vector types, we do not default to the mode's alignment.
! 	   Instead, query a target hook, defaulting to natural alignment.
! 	   This prevents ABI changes depending on whether or not native
! 	   vector modes are supported.  */
! 	TYPE_ALIGN (type) = targetm.vector_alignment (type);
! 
! 	/* However, if the underlying mode requires a bigger alignment than
! 	   what the target hook provides, we cannot use the mode.  For now,
! 	   simply reject that case.  */
! 	gcc_assert (TYPE_ALIGN (type)
! 		    >= GET_MODE_ALIGNMENT (TYPE_MODE (type)));
          break;
        }
  
Index: gcc/tree-vect-data-refs.c
===================================================================
*** gcc/tree-vect-data-refs.c	(revision 190202)
--- gcc/tree-vect-data-refs.c	(working copy)
*************** vect_update_misalignment_for_peel (struc
*** 1019,1025 ****
        int misal = DR_MISALIGNMENT (dr);
        tree vectype = STMT_VINFO_VECTYPE (stmt_info);
        misal += negative ? -npeel * dr_size : npeel * dr_size;
!       misal &= GET_MODE_SIZE (TYPE_MODE (vectype)) - 1;
        SET_DR_MISALIGNMENT (dr, misal);
        return;
      }
--- 1019,1025 ----
        int misal = DR_MISALIGNMENT (dr);
        tree vectype = STMT_VINFO_VECTYPE (stmt_info);
        misal += negative ? -npeel * dr_size : npeel * dr_size;
!       misal &= (TYPE_ALIGN (vectype) / BITS_PER_UNIT) - 1;
        SET_DR_MISALIGNMENT (dr, misal);
        return;
      }
Index: gcc/config/arm/arm.c
===================================================================
*** gcc/config/arm/arm.c	(revision 190202)
--- gcc/config/arm/arm.c	(working copy)
*************** static bool xscale_sched_adjust_cost (rt
*** 243,248 ****
--- 243,249 ----
  static bool fa726te_sched_adjust_cost (rtx, rtx, rtx, int *);
  static enum machine_mode arm_preferred_simd_mode (enum machine_mode);
  static bool arm_class_likely_spilled_p (reg_class_t);
+ static HOST_WIDE_INT arm_vector_alignment (const_tree type);
  static bool arm_vector_alignment_reachable (const_tree type, bool is_packed);
  static bool arm_builtin_support_vector_misalignment (enum machine_mode mode,
  						     const_tree type,
*************** static const struct default_options arm_
*** 579,584 ****
--- 580,588 ----
  #undef TARGET_CLASS_LIKELY_SPILLED_P
  #define TARGET_CLASS_LIKELY_SPILLED_P arm_class_likely_spilled_p
  
+ #undef TARGET_VECTOR_ALIGNMENT
+ #define TARGET_VECTOR_ALIGNMENT arm_vector_alignment
+ 
  #undef TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE
  #define TARGET_VECTORIZE_VECTOR_ALIGNMENT_REACHABLE \
    arm_vector_alignment_reachable
*************** arm_function_arg (CUMULATIVE_ARGS *pcum,
*** 4693,4698 ****
--- 4697,4714 ----
    return gen_rtx_REG (mode, pcum->nregs);
  }
  
+ /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
+ static HOST_WIDE_INT
+ arm_vector_alignment (const_tree type)
+ {
+   HOST_WIDE_INT align = tree_low_cst (TYPE_SIZE (type), 0);
+ 
+   if (TARGET_AAPCS_BASED)
+     align = MIN (align, 64);
+ 
+   return align;
+ }
+ 
  static unsigned int
  arm_function_arg_boundary (enum machine_mode mode, const_tree type)
  {


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFC] Target-specific limits on vector alignment

[Hans-Peter Nilsson]

> From: Richard Earnshaw <rearnsha@arm.com>
> Date: Mon, 11 Jun 2012 15:16:50 +0200

> The ARM ABI states that vectors larger than 64 bits in size still have
> 64-bit alignment; never-the-less, the HW supports alignment hints of up
> to 128-bits in some cases and will trap in a vector has an alignment
> that less than the hint.  GCC currently hard-codes larger vectors to be
> aligned by the size of the vector, which means that 128-bit vectors are
> marked as being 128-bit aligned.
> 
> The ARM ABI unfortunately does not support generating such alignment for
> parameters passed by value and this can lead to traps at run time.  It
> seems that the best way to solve this problem is to allow the back-end
> to set an upper limit on the alignment permitted for a vector.

Don't you mean lowering the limit on the alignment required and
guaranteed for a vector?  (Or else it sounds like you want to
force attribute-aligned to refuse higher values.)

Wouldn't it be better to make the */*modes.def ADJUST_ALIGNMENT
macro actually work?  Does it work for you?  The epiphany port
tries to do it that way (optionally, not the default AFAICT),
but I bet it's actually unsuccessful.

> I've implemented this as a separate hook, rather than using the existing
> hooks because there's a strong likelihood of breaking some existing ABIs
> if I did it another way.
> 
> There are a couple of tests that will need some re-working before this
> can be committed to deal with the fall-out of making this change; I'll
> prepare those changes if this patch is deemed generally acceptable.

It'd be nice to have non-naturally-aligned vectors working in
general.

I have a MIPS COP2 128-bit-SIMD implementation for 32-bit MIPS,
which means 64-bit stack-alignment and memory loads and stores
are in 64-bit parts.  So, I've chosen to not require any larger
alignment to keep ABI compatibility (vectors aren't passed in
registers; core use is intra-function anyway).  This mostly
works, but there's pushback from gcc (at least in 4.3): trying
to adjust vector-type-alignment by e.g. "ADJUST_ALIGNMENT (V4SI,
8);" in mips-modes.def has no effect; I had to do it in
mips_builtin_vector_type, which might be brittle or just working
by chance.  Then there's the tree-vectorizer, which for some
optimizations wants to assume natural alignment when doing
"floor alignment".  I couldn't define a usable
"vec_realign_load_<mode>" (undocumented) as it assumes that the
low vector-size bits of an address will exactly correspond to
misalignment when masking off and applying to a vector.  And,
you may have to conditionalize some vectorizer tests with the
effective-target unaligned_stack.

Just preparing you for the fall-out. :]

brgds, H-P

Re: [RFC] Target-specific limits on vector alignment

[Ulrich Weigand]

Hans-Peter Nilsson wrote:
> > From: Richard Earnshaw <rearnsha@arm.com>
> > Date: Mon, 11 Jun 2012 15:16:50 +0200
> 
> > The ARM ABI states that vectors larger than 64 bits in size still have
> > 64-bit alignment; never-the-less, the HW supports alignment hints of up
> > to 128-bits in some cases and will trap in a vector has an alignment
> > that less than the hint.  GCC currently hard-codes larger vectors to be
> > aligned by the size of the vector, which means that 128-bit vectors are
> > marked as being 128-bit aligned.
> > 
> > The ARM ABI unfortunately does not support generating such alignment for
> > parameters passed by value and this can lead to traps at run time.  It
> > seems that the best way to solve this problem is to allow the back-end
> > to set an upper limit on the alignment permitted for a vector.
> 
> Don't you mean lowering the limit on the alignment required and
> guaranteed for a vector?  (Or else it sounds like you want to
> force attribute-aligned to refuse higher values.)
> 
> Wouldn't it be better to make the */*modes.def ADJUST_ALIGNMENT
> macro actually work?  Does it work for you?  The epiphany port
> tries to do it that way (optionally, not the default AFAICT),
> but I bet it's actually unsuccessful.

I've started looking into that as well now.  Actually, there's two
different issues: the alignment of vector *modes*, and the alignment
of vector *types*.

As to vector modes, they default to natural alignment, which can be
overridden via ADJUST_ALIGNMENT.  In either case, it is then capped
by BIGGEST_ALIGNMENT.  (Note that this means e.g. on ARM, 16-byte
vector *modes* are already effectively not naturally aligned, since
BIGGEST_ALIGNMENT is set to 64.)

A different issue is the alignment of vector *types*.  While usually
scalar types inherit their alignment from the underlying machine mode
used to implement the type, this is deliberately not done for vector
types (see layout_type).  The reason for that is sometimes, there may
be an underlying machine mode to represent a vector types, and at
other times (maybe just because of a different -march setting), there
may *not* be such an underlying machine mode.  Since it would be a
bad idea to have vector type alignment (an ABI property!) depend on
-march flags, layout_type instead hard-codes natural alignment for
all vector types.

> > I've implemented this as a separate hook, rather than using the existing
> > hooks because there's a strong likelihood of breaking some existing ABIs
> > if I did it another way.
> > 
> > There are a couple of tests that will need some re-working before this
> > can be committed to deal with the fall-out of making this change; I'll
> > prepare those changes if this patch is deemed generally acceptable.

That hook changes the alignment requirement for vector types.  However,
those will still be *increased* in finalize_type_size to the alignment
of an underlying vector mode (if any), if that is greater.

Fortunately, this does not occur in the ARM case since vector mode
alignment is bounded by BIGGEST_ALIGNMENT, and the ARM implementation
of the new hook never returns anything smaller.  However, for the
general case this would re-introduce the possibility that vector
type alignment differs based on the presence or absence of vector
modes.

Maybe we need to -in addition- refuse to use a vector mode if it has
a bigger alignment requirement than the vector type alignment
determined by the hook?  Something along the lines of what is done
in compute_record_mode:

  /* If structure's known alignment is less than what the scalar
     mode would need, and it matters, then stick with BLKmode.  */
  if (TYPE_MODE (type) != BLKmode
      && STRICT_ALIGNMENT
      && ! (TYPE_ALIGN (type) >= BIGGEST_ALIGNMENT
            || TYPE_ALIGN (type) >= GET_MODE_ALIGNMENT (TYPE_MODE (type))))
    {
      /* If this is the only reason this type is BLKmode, then
         don't force containing types to be BLKmode.  */
      TYPE_NO_FORCE_BLK (type) = 1;
      SET_TYPE_MODE (type, BLKmode);
    }

[ If a target still wants vector modes to be used, it needs to *also*
reduce their alignment requirements via ADJUST_ALIGNMENT. ]


> It'd be nice to have non-naturally-aligned vectors working in
> general.
> 
> I have a MIPS COP2 128-bit-SIMD implementation for 32-bit MIPS,
> which means 64-bit stack-alignment and memory loads and stores
> are in 64-bit parts.  So, I've chosen to not require any larger
> alignment to keep ABI compatibility (vectors aren't passed in
> registers; core use is intra-function anyway).  This mostly
> works, but there's pushback from gcc (at least in 4.3): trying
> to adjust vector-type-alignment by e.g. "ADJUST_ALIGNMENT (V4SI,
> 8);" in mips-modes.def has no effect; I had to do it in
> mips_builtin_vector_type, which might be brittle or just working
> by chance.

See above as to why ADJUST_ALIGNMENT it itself doesn't change the
alignment of vector types.  mips_builtin_vector_type affects only
the types created by the back-end, not generic vector types e.g.
the ones generated by the vectorizer on the fly ...

> Then there's the tree-vectorizer, which for some
> optimizations wants to assume natural alignment when doing
> "floor alignment".  I couldn't define a usable
> "vec_realign_load_<mode>" (undocumented) as it assumes that the
> low vector-size bits of an address will exactly correspond to
> misalignment when masking off and applying to a vector.  And,
> you may have to conditionalize some vectorizer tests with the
> effective-target unaligned_stack.

I *think* this ought to work out OK, since the realignment scheme
consults the type alignment:

      new_stmt = gimple_build_assign_with_ops
                   (BIT_AND_EXPR, NULL_TREE, ptr,
                    build_int_cst (TREE_TYPE (ptr),
                                   -(HOST_WIDE_INT)TYPE_ALIGN_UNIT (vectype)));

So once TYPE_ALIGN is actually set up correctly, this hopefully
will just work as expected ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFC] Target-specific limits on vector alignment

[Hans-Peter Nilsson]

> From: Ulrich Weigand <uweigand@de.ibm.com>
> Date: Tue, 24 Jul 2012 19:38:15 +0200

> > > I've implemented this as a separate hook, rather than using the existing
> > > hooks because there's a strong likelihood of breaking some existing ABIs
> > > if I did it another way.
> > > 
> > > There are a couple of tests that will need some re-working before this
> > > can be committed to deal with the fall-out of making this change; I'll
> > > prepare those changes if this patch is deemed generally acceptable.
> 
> That hook changes the alignment requirement for vector types.  However,
> those will still be *increased* in finalize_type_size to the alignment
> of an underlying vector mode (if any), if that is greater.
> 
> Fortunately, this does not occur in the ARM case since vector mode
> alignment is bounded by BIGGEST_ALIGNMENT, and the ARM implementation
> of the new hook never returns anything smaller.  However, for the
> general case this would re-introduce the possibility that vector
> type alignment differs based on the presence or absence of vector
> modes.

I don't follow all your arguments right now; I'll have to
revisit this in abut three weeks, but what's true for ARM should
be true for MIPS in my case (32-bit o32 with vendor-specific
16-byte vectors): BIGGEST_ALIGNMENT is the same 64 bits
(non-aligned for 16-byte vectors) AFAICS.  Nice to know they
have the same issue and I have something to compare. :)

brgds, H-P