Patchwork DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)

login
register
mail settings
Submitter Alan Modra
Date June 13, 2013, 7:40 a.m.
Message ID <20130613074051.GG21523@bubble.grove.modra.org>
Download mbox | patch
Permalink /patch/251000/
State New
Headers show

Comments

Alan Modra - June 13, 2013, 7:40 a.m.
On Wed, Jun 12, 2013 at 12:52:03PM -0500, Edmar Wienskoski wrote:
> The e500v2 (SPE) hardware is such that if the address of vector (double world
> load / stores) are not double world aligned the instruction will trap.
> 
> So this alignment is not optional.

Vector type alignment is also specified by the ppc64 abi.  I think we
want the following.  Note that DATA_ALIGNMENT has been broken for
vectors right from the initial vector support (and the error was
copied for e500 double).  For example

typedef int vec_align __attribute__ ((vector_size(16), aligned(32)));
vec_align x = { 0, 0, 0, 0 };

currently loses the extra alignment.  Fixed by never decreasing
alignment in DATA_ABI_ALIGNMENT.  Testing in progress.  OK to
apply assuming bootstrap is good?  (I think I need a change in
offsettable_ok_by_alignment too.  I'll do that in a separate patch.)

	* config/rs6000/rs6000.h (DATA_ABI_ALIGNMENT): Define.
	(DATA_ALIGNMENT): Remove alignment already covered by above.
	(LOCAL_ALIGNMENT): Use both DATA_ABI_ALIGNMENT and DATA_ALIGNMENT.

Patch

Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 200055)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -817,7 +817,8 @@  extern unsigned rs6000_pointer_size;
    local store.  TYPE is the data type, and ALIGN is the alignment
    that the object would ordinarily have.  */
 #define LOCAL_ALIGNMENT(TYPE, ALIGN)				\
-  DATA_ALIGNMENT (TYPE, ALIGN)
+  ({unsigned int _align = DATA_ABI_ALIGNMENT (TYPE, ALIGN);	\
+    DATA_ALIGNMENT (TYPE, _align); })
 
 /* Alignment of field after `int : 0' in a structure.  */
 #define EMPTY_FIELD_BOUNDARY 32
@@ -837,21 +838,26 @@  extern unsigned rs6000_pointer_size;
    ? BITS_PER_WORD                                               \
    : (ALIGN))
 
-/* Make arrays of chars word-aligned for the same reasons.
-   Align vectors to 128 bits.  Align SPE vectors and E500 v2 doubles to
+/* Make arrays of chars word-aligned for the same reasons.  */
+#define DATA_ALIGNMENT(TYPE, ALIGN)					\
+  ((TREE_CODE (TYPE) == ARRAY_TYPE					\
+    && TYPE_MODE (TREE_TYPE (TYPE)) == QImode				\
+    && (ALIGN) < BITS_PER_WORD) ? BITS_PER_WORD : (ALIGN))
+
+/* Align vectors to 128 bits.  Align SPE vectors and E500 v2 doubles to
    64 bits.  */
-#define DATA_ALIGNMENT(TYPE, ALIGN)					\
+#define DATA_ABI_ALIGNMENT(TYPE, ALIGN)					\
   (TREE_CODE (TYPE) == VECTOR_TYPE					\
    ? (((TARGET_SPE && SPE_VECTOR_MODE (TYPE_MODE (TYPE)))		\
        || (TARGET_PAIRED_FLOAT && PAIRED_VECTOR_MODE (TYPE_MODE (TYPE)))) \
-      ? 64 : 128)							\
-   : ((TARGET_E500_DOUBLE						\
-       && TREE_CODE (TYPE) == REAL_TYPE					\
-       && TYPE_MODE (TYPE) == DFmode)					\
-      ? 64								\
-      : (TREE_CODE (TYPE) == ARRAY_TYPE					\
-	 && TYPE_MODE (TREE_TYPE (TYPE)) == QImode			\
-	 && (ALIGN) < BITS_PER_WORD) ? BITS_PER_WORD : (ALIGN)))
+      ? ((ALIGN) < 64 ? 64 : (ALIGN))					\
+      : ((ALIGN) < 128 ? 128 : (ALIGN)))				\
+   : (TARGET_E500_DOUBLE						\
+      && TREE_CODE (TYPE) == REAL_TYPE					\
+      && TYPE_MODE (TYPE) == DFmode					\
+      && (ALIGN) < 64)							\
+   ? 64									\
+   : (ALIGN))
 
 /* Nonzero if move instructions will actually fail to work
    when given unaligned data.  */