[PATCH 0/2][trunk+5 backport][ARM] PR/65956 Implement AAPCS updates for alignment attribute

[PATCH 0/2][trunk+5 backport][ARM] PR/65956 Implement AAPCS updates for alignment attribute

[Alan Lawrence]

This patch series implements the changes/additions to the ARM ABI proposed at 
https://gcc.gnu.org/ml/gcc/2015-07/msg00040.html .

The first patch is the ABI update. This is an ABI-breaking change for any code 
using __attribute__((aligned(...))) on a public interface (a case not previously 
defined by the AAPCS).

This causes a regression of gcc.c-torture/execute/20040709-1.c at -O0 (only), 
and the align_rec2.c fails, both due to a latent bug where we can emit strd/ldrd 
on an odd-numbered register in ARM state. The second patch prevents such illegal 
instructions and fixes both tests.

On trunk, tested via bootstrap + check-gcc on arm-none-linux-gnueabihf 
(cortex-a15+neon). Also cross-tested arm-none-eabi with a number of variants.

On gcc-5-branch, patches rebase cleanly, tested via profiledbootstrap + 
check-gcc. (Yes, profiledbootstrap succeeds.)

[PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

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

These include tests of structs, scalars, and vectors - only general-purpose 
registers are affected by the ABI rules for alignment, but we can restrict the 
vector test to use the base AAPCS.

Prior to this patch, align2.c, align3.c and align_rec1.c were failing (the 
latter showing an internal inconsistency, the first two merely that GCC did not 
obey the new ABI).

With this patch, the align_rec2.c fails, and also 
gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a latent bug 
where we can emit strd/ldrd on an odd-numbered register in ARM state, fixed by 
the second patch.

gcc/ChangeLog:

	* config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
	alignment attribute, exploring one level down for aggregates.

gcc/testsuite/ChangeLog:

	* gcc.target/arm/aapcs/align1.c: New.
	* gcc.target/arm/aapcs/align_rec1.c: New.
	* gcc.target/arm/aapcs/align2.c: New.
	* gcc.target/arm/aapcs/align_rec2.c: New.
	* gcc.target/arm/aapcs/align3.c: New.
	* gcc.target/arm/aapcs/align_rec3.c: New.
	* gcc.target/arm/aapcs/align4.c: New.
	* gcc.target/arm/aapcs/align_rec4.c: New.
	* gcc.target/arm/aapcs/align_vararg1.c: New.
	* gcc.target/arm/aapcs/align_vararg2.c: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm_overalign_1.patch --]
[-- Type: text/x-patch; name=arm_overalign_1.patch, Size: 12365 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS *pcum, tree fntype,
 static bool
 arm_needs_doubleword_align (machine_mode mode, const_tree type)
 {
-  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
-	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
+  if (!type)
+    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
+
+  if (!AGGREGATE_TYPE_P (type))
+    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
+
+  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
+    if (DECL_ALIGN (field) > PARM_BOUNDARY)
+      return true;
+
+  return false;
 }
 
 
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
new file mode 100644
index 0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
@@ -0,0 +1,29 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "align1.c"
+
+typedef __attribute__((aligned (8))) int alignedint;
+
+alignedint a = 11;
+alignedint b = 13;
+alignedint c = 17;
+alignedint d = 19;
+alignedint e = 23;
+alignedint f = 29;
+
+#include "abitest.h"
+#else
+  ARG (alignedint, a, R0)
+  /* Attribute suggests R2, but we should use only natural alignment:  */
+  ARG (alignedint, b, R1)
+  ARG (alignedint, c, R2)
+  ARG (alignedint, d, R3)
+  ARG (alignedint, e, STACK)
+  /* Attribute would suggest STACK + 8 but should be ignored:  */
+  LAST_ARG (alignedint, f, STACK + 4)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
new file mode 100644
index 0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
@@ -0,0 +1,30 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "align2.c"
+
+/* The underlying struct here has alignment 4.  */
+typedef struct __attribute__((aligned (8)))
+  {
+    int x;
+    int y;
+  } overaligned;
+
+/* A couple of instances, at 8-byte-aligned memory locations.  */
+overaligned a = { 2, 3 };
+overaligned b = { 5, 8 };
+
+#include "abitest.h"
+#else
+  ARG (int, 7, R0)
+  /* Alignment should be 4.  */
+  ARG (overaligned, a, R1)
+  ARG (int, 9, R3)
+  ARG (int, 10, STACK)
+  /* Alignment should be 4.  */
+  LAST_ARG (overaligned, b, STACK + 4)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
new file mode 100644
index 0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
@@ -0,0 +1,42 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O3" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "align3.c"
+
+/* Struct will be aligned to 8.  */
+struct s
+  {
+    int x;
+    /* 4 bytes padding here.  */
+    __attribute__((aligned (8))) int y;
+    /* 4 bytes padding here.  */
+  };
+
+typedef struct s __attribute__((aligned (4))) underaligned;
+
+#define EXPECTED_STRUCT_SIZE 16
+extern void link_failure (void);
+int
+foo ()
+{
+  /* Optimization gets rid of this before linking.  */
+  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
+    link_failure ();
+}
+
+underaligned a = { 1, 4 };
+underaligned b = { 9, 16 };
+
+#include "abitest.h"
+#else
+  ARG (int, 3, R0)
+  /* Object alignment is 8, so split between 2 regs and 8 on stack.  */
+  ARG (underaligned, a, R2)
+  ARG (int, 6, STACK + 8)
+  /* Object alignment is 8, so skip over STACK + 12.  */
+  LAST_ARG (underaligned, b, STACK + 16)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
new file mode 100644
index 0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
@@ -0,0 +1,29 @@
+/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-require-effective-target arm_neon_ok  } */
+/* { dg-options "-O" } */
+/* { dg-add-options arm_neon } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "align4.c"
+
+#define PCSATTR __attribute__((pcs("aapcs")))
+
+#include <arm_neon.h>
+
+typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
+
+unalignedvec a = {11, 13};
+unalignedvec b = {17, 19};
+
+#include "abitest.h"
+#else
+  ARG (int, 2, R0)
+  /* Attribute suggests R1, but we should use natural alignment:  */
+  ARG (unalignedvec, a, R2)
+  ARG (int, 6, STACK)
+  /* Attribute would suggest STACK + 4 but should be ignored:  */
+  LAST_ARG (unalignedvec, b, STACK + 8)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
new file mode 100644
index 0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
@@ -0,0 +1,36 @@
+/* Test AAPCS layout (alignment) for callee.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O2 -fno-inline" } */
+
+extern void abort (void);
+
+typedef __attribute__((aligned (8))) int alignedint;
+
+alignedint a = 11;
+alignedint b = 13;
+alignedint c = 17;
+alignedint d = 19;
+alignedint e = 23;
+alignedint f = 29;
+
+void
+foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
+     alignedint stack, alignedint stack4)
+{
+  if (r0 != a
+      || r1 != b
+      || r2 != c
+      || r3 != d
+      || stack != e
+      || stack4 !=f)
+    abort ();
+}
+
+int
+main (int argc, char **argv)
+{
+  foo (a, b, c, d, e, f);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
new file mode 100644
index 0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
@@ -0,0 +1,41 @@
+/* Test AAPCS layout (alignment) for callee.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O2 -fno-inline" } */
+
+extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
+extern void abort (void);
+
+typedef struct __attribute__((aligned (8)))
+  {
+    int x;
+    int y;
+  } overaligned;
+
+overaligned a = { 2, 3 };
+overaligned b = { 5, 8 };
+
+void
+f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
+{
+  if (r0 != 7 || r3 != 9 || stack != 10)
+    abort ();
+  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
+    abort ();
+  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
+    abort ();
+  int addr = ((int) &stack4) & 7;
+  if (addr != 0)
+    {
+      __builtin_printf ("Alignment was %d\n", addr);
+      abort ();
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  f (7, a, 9, 10, b);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
new file mode 100644
index 0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
@@ -0,0 +1,43 @@
+/* Test AAPCS layout (alignment) for callee.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O2 -fno-inline" } */
+
+/* Test AAPCS layout (alignment) for callee.  */
+
+extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
+extern void abort (void);
+
+
+/* Struct will be aligned to 8.  */
+struct s
+  {
+    int x;
+    /* 4 bytes padding here.  */
+    __attribute__((aligned (8))) int y;
+    /* 4 bytes padding here.  */
+  };
+
+typedef struct s __attribute__((aligned (4))) underaligned;
+
+underaligned a = { 1, 4 };
+underaligned b = { 9, 16 };
+
+void
+f (int r0, underaligned r2, int stack8, underaligned stack16)
+{
+  if (r0 != 3 || stack8 != 6)
+    abort ();
+  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
+    abort ();
+  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
+    abort ();
+}
+
+int
+main (int argc, char **argv)
+{
+  f (3, a, 6, b);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
new file mode 100644
index 0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
@@ -0,0 +1,33 @@
+/* Test AAPCS layout (alignment) for callee.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O -fno-inline" } */
+/* { dg-add-options arm_neon } */
+
+#include <arm_neon.h>
+
+extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
+extern void abort (void);
+
+typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
+
+unalignedvec a = {11, 13};
+unalignedvec b = {17, 19};
+
+void
+foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
+{
+  if (r0 != 2 || s0 != 6
+      || memcmp ( (void *) &r2, (void *) &a, 16)
+      || memcmp ( (void *) &s8, (void *) &b, 16))
+    abort ();
+}
+
+int
+main (int argc, char **argv)
+{
+  foo (2, a, 6, b);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
@@ -0,0 +1,36 @@
+/* Test AAPCS layout (alignment of varargs) for callee.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O2 -fno-inline" } */
+
+#include <stdarg.h>
+
+extern void abort (void);
+
+typedef __attribute__((aligned (8))) int alignedint;
+
+void
+foo (int i, ...)
+{
+  va_list va;
+  va_start (va, i);
+  /* Arguments should be passed in the same registers as if they were ints.  */
+  while (i-- > 0)
+    if (va_arg (va, int) != i)
+      abort ();
+  va_end (va);
+}
+
+int
+main (int argc, char **argv)
+{
+  alignedint a = 5;
+  alignedint b = 4;
+  alignedint c = 3;
+  alignedint d = 2;
+  alignedint e = 1;
+  alignedint f = 0;
+  foo (a, b, c, d, e, f);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
@@ -0,0 +1,30 @@
+/* Test AAPCS layout (alignment of varargs) for callee.  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O2 -fno-inline" } */
+
+#include <stdarg.h>
+
+extern void abort (void);
+
+typedef __attribute__((aligned (8))) int alignedint;
+
+void
+foo (int i, ...)
+{
+  va_list va;
+  va_start (va, i);
+  /* alignedint should be pulled out of regs/stack just like an int.  */
+  while (i-- > 0)
+    if (va_arg (va, alignedint) != i)
+      abort ();
+  va_end (va);
+}
+
+int
+main (int argc, char **argv)
+{
+  foo (5, 4, 3, 2, 1, 0);
+  return 0;
+}

[PATCH 2/2][ARM] fix movdi expander to avoid illegal ldrd/strd

[Alan Lawrence]

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

The previous patch caused a regression in gcc.c-torture/execute/20040709-1.c at 
-O0 (only), and the new align_rec2.c test fails, both outputting an illegal 
assembler instruction (ldrd on an odd-numbered reg) from output_move_double in 
arm.c. Most routes have checks against such an illegal instruction, but 
expanding a function call can directly name such impossible register (pairs), 
bypassing the normal checks.

gcc/ChangeLog:

	* config/arm/arm.md (movdi): Avoid odd-number ldrd/strd in ARM state.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm_overalign_2.patch --]
[-- Type: text/x-patch; name=arm_overalign_2.patch, Size: 1894 bytes --]

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 164ac13a26289bf755c89e78a8a5f751883c6039..c6718282d2555f8cf9a4e9111b1393e1f7704983 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -5415,6 +5415,42 @@
       if (!REG_P (operands[0]))
 	operands[1] = force_reg (DImode, operands[1]);
     }
+  if (REG_P (operands[0]) && REGNO (operands[0]) < FIRST_VIRTUAL_REGISTER
+      && !HARD_REGNO_MODE_OK (REGNO (operands[0]), DImode))
+    {
+      /* Avoid LDRD's into an odd-numbered register pair in ARM state
+	 when expanding function calls.  */
+      gcc_assert (can_create_pseudo_p ());
+      if (MEM_P (operands[1]) && MEM_VOLATILE_P (operands[1]))
+	{
+	  /* Perform load into legal reg pair first, then move.  */
+	  rtx reg = gen_reg_rtx (DImode);
+	  emit_insn (gen_movdi (reg, operands[1]));
+	  operands[1] = reg;
+	}
+      emit_move_insn (gen_lowpart (SImode, operands[0]),
+		      gen_lowpart (SImode, operands[1]));
+      emit_move_insn (gen_highpart (SImode, operands[0]),
+	      gen_highpart (SImode, operands[1]));
+      DONE;
+    }
+  else if (REG_P (operands[1]) && REGNO (operands[1]) < FIRST_VIRTUAL_REGISTER
+	   && !HARD_REGNO_MODE_OK (REGNO (operands[1]), DImode))
+    {
+      /* Avoid LDRD's into an odd-numbered register pair in ARM state
+	 when expanding function prologue.  */
+      gcc_assert (can_create_pseudo_p ());
+      rtx split_dest = (MEM_P (operands[0]) && MEM_VOLATILE_P (operands[0]))
+		       ? gen_reg_rtx (DImode)
+		       : operands[0];
+      emit_move_insn (gen_lowpart (SImode, split_dest),
+		      gen_lowpart (SImode, operands[1]));
+      emit_move_insn (gen_highpart (SImode, split_dest),
+	      gen_highpart (SImode, operands[1]));
+      if (split_dest != operands[0])
+	emit_insn (gen_movdi (operands[0], split_dest));
+      DONE;
+    }
   "
 )
 

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Richard Earnshaw]

On 03/07/15 16:26, Alan Lawrence wrote:
> These include tests of structs, scalars, and vectors - only
> general-purpose registers are affected by the ABI rules for alignment,
> but we can restrict the vector test to use the base AAPCS.
> 
> Prior to this patch, align2.c, align3.c and align_rec1.c were failing
> (the latter showing an internal inconsistency, the first two merely that
> GCC did not obey the new ABI).
> 
> With this patch, the align_rec2.c fails, and also
> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a latent
> bug where we can emit strd/ldrd on an odd-numbered register in ARM
> state, fixed by the second patch.
> 
> gcc/ChangeLog:
> 
>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>     alignment attribute, exploring one level down for aggregates.
> 
> gcc/testsuite/ChangeLog:
> 
>     * gcc.target/arm/aapcs/align1.c: New.
>     * gcc.target/arm/aapcs/align_rec1.c: New.
>     * gcc.target/arm/aapcs/align2.c: New.
>     * gcc.target/arm/aapcs/align_rec2.c: New.
>     * gcc.target/arm/aapcs/align3.c: New.
>     * gcc.target/arm/aapcs/align_rec3.c: New.
>     * gcc.target/arm/aapcs/align4.c: New.
>     * gcc.target/arm/aapcs/align_rec4.c: New.
>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>     * gcc.target/arm/aapcs/align_vararg2.c: New.
> 
> arm_overalign_1.patch
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS *pcum, tree fntype,
>  static bool
>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>  {
> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
> +  if (!type)
> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
> +
> +  if (!AGGREGATE_TYPE_P (type))
> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
> +
> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
> +      return true;
> +

Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
case though (unless there's a language that allows passing arrays by value).

For array types I think you need to check TYPE_ALIGN (TREE_TYPE (type)).

R.

> +  return false;
>  }
>  
>  
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
> @@ -0,0 +1,29 @@
> +/* Test AAPCS layout (alignment).  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O" } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align1.c"
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +alignedint a = 11;
> +alignedint b = 13;
> +alignedint c = 17;
> +alignedint d = 19;
> +alignedint e = 23;
> +alignedint f = 29;
> +
> +#include "abitest.h"
> +#else
> +  ARG (alignedint, a, R0)
> +  /* Attribute suggests R2, but we should use only natural alignment:  */
> +  ARG (alignedint, b, R1)
> +  ARG (alignedint, c, R2)
> +  ARG (alignedint, d, R3)
> +  ARG (alignedint, e, STACK)
> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
> +  LAST_ARG (alignedint, f, STACK + 4)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
> @@ -0,0 +1,30 @@
> +/* Test AAPCS layout (alignment).  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O" } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align2.c"
> +
> +/* The underlying struct here has alignment 4.  */
> +typedef struct __attribute__((aligned (8)))
> +  {
> +    int x;
> +    int y;
> +  } overaligned;
> +
> +/* A couple of instances, at 8-byte-aligned memory locations.  */
> +overaligned a = { 2, 3 };
> +overaligned b = { 5, 8 };
> +
> +#include "abitest.h"
> +#else
> +  ARG (int, 7, R0)
> +  /* Alignment should be 4.  */
> +  ARG (overaligned, a, R1)
> +  ARG (int, 9, R3)
> +  ARG (int, 10, STACK)
> +  /* Alignment should be 4.  */
> +  LAST_ARG (overaligned, b, STACK + 4)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
> @@ -0,0 +1,42 @@
> +/* Test AAPCS layout (alignment).  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O3" } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align3.c"
> +
> +/* Struct will be aligned to 8.  */
> +struct s
> +  {
> +    int x;
> +    /* 4 bytes padding here.  */
> +    __attribute__((aligned (8))) int y;
> +    /* 4 bytes padding here.  */
> +  };
> +
> +typedef struct s __attribute__((aligned (4))) underaligned;
> +
> +#define EXPECTED_STRUCT_SIZE 16
> +extern void link_failure (void);
> +int
> +foo ()
> +{
> +  /* Optimization gets rid of this before linking.  */
> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
> +    link_failure ();
> +}
> +
> +underaligned a = { 1, 4 };
> +underaligned b = { 9, 16 };
> +
> +#include "abitest.h"
> +#else
> +  ARG (int, 3, R0)
> +  /* Object alignment is 8, so split between 2 regs and 8 on stack.  */
> +  ARG (underaligned, a, R2)
> +  ARG (int, 6, STACK + 8)
> +  /* Object alignment is 8, so skip over STACK + 12.  */
> +  LAST_ARG (underaligned, b, STACK + 16)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
> @@ -0,0 +1,29 @@
> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-require-effective-target arm_neon_ok  } */
> +/* { dg-options "-O" } */
> +/* { dg-add-options arm_neon } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align4.c"
> +
> +#define PCSATTR __attribute__((pcs("aapcs")))
> +
> +#include <arm_neon.h>
> +
> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
> +
> +unalignedvec a = {11, 13};
> +unalignedvec b = {17, 19};
> +
> +#include "abitest.h"
> +#else
> +  ARG (int, 2, R0)
> +  /* Attribute suggests R1, but we should use natural alignment:  */
> +  ARG (unalignedvec, a, R2)
> +  ARG (int, 6, STACK)
> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
> +  LAST_ARG (unalignedvec, b, STACK + 8)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
> @@ -0,0 +1,36 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +alignedint a = 11;
> +alignedint b = 13;
> +alignedint c = 17;
> +alignedint d = 19;
> +alignedint e = 23;
> +alignedint f = 29;
> +
> +void
> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
> +     alignedint stack, alignedint stack4)
> +{
> +  if (r0 != a
> +      || r1 != b
> +      || r2 != c
> +      || r3 != d
> +      || stack != e
> +      || stack4 !=f)
> +    abort ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  foo (a, b, c, d, e, f);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
> @@ -0,0 +1,41 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
> +extern void abort (void);
> +
> +typedef struct __attribute__((aligned (8)))
> +  {
> +    int x;
> +    int y;
> +  } overaligned;
> +
> +overaligned a = { 2, 3 };
> +overaligned b = { 5, 8 };
> +
> +void
> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
> +{
> +  if (r0 != 7 || r3 != 9 || stack != 10)
> +    abort ();
> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
> +    abort ();
> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
> +    abort ();
> +  int addr = ((int) &stack4) & 7;
> +  if (addr != 0)
> +    {
> +      __builtin_printf ("Alignment was %d\n", addr);
> +      abort ();
> +    }
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  f (7, a, 9, 10, b);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
> @@ -0,0 +1,43 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
> +extern void abort (void);
> +
> +
> +/* Struct will be aligned to 8.  */
> +struct s
> +  {
> +    int x;
> +    /* 4 bytes padding here.  */
> +    __attribute__((aligned (8))) int y;
> +    /* 4 bytes padding here.  */
> +  };
> +
> +typedef struct s __attribute__((aligned (4))) underaligned;
> +
> +underaligned a = { 1, 4 };
> +underaligned b = { 9, 16 };
> +
> +void
> +f (int r0, underaligned r2, int stack8, underaligned stack16)
> +{
> +  if (r0 != 3 || stack8 != 6)
> +    abort ();
> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
> +    abort ();
> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
> +    abort ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  f (3, a, 6, b);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
> @@ -0,0 +1,33 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-O -fno-inline" } */
> +/* { dg-add-options arm_neon } */
> +
> +#include <arm_neon.h>
> +
> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
> +
> +unalignedvec a = {11, 13};
> +unalignedvec b = {17, 19};
> +
> +void
> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
> +{
> +  if (r0 != 2 || s0 != 6
> +      || memcmp ( (void *) &r2, (void *) &a, 16)
> +      || memcmp ( (void *) &s8, (void *) &b, 16))
> +    abort ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  foo (2, a, 6, b);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
> @@ -0,0 +1,36 @@
> +/* Test AAPCS layout (alignment of varargs) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +#include <stdarg.h>
> +
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +void
> +foo (int i, ...)
> +{
> +  va_list va;
> +  va_start (va, i);
> +  /* Arguments should be passed in the same registers as if they were ints.  */
> +  while (i-- > 0)
> +    if (va_arg (va, int) != i)
> +      abort ();
> +  va_end (va);
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  alignedint a = 5;
> +  alignedint b = 4;
> +  alignedint c = 3;
> +  alignedint d = 2;
> +  alignedint e = 1;
> +  alignedint f = 0;
> +  foo (a, b, c, d, e, f);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
> @@ -0,0 +1,30 @@
> +/* Test AAPCS layout (alignment of varargs) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +#include <stdarg.h>
> +
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +void
> +foo (int i, ...)
> +{
> +  va_list va;
> +  va_start (va, i);
> +  /* alignedint should be pulled out of regs/stack just like an int.  */
> +  while (i-- > 0)
> +    if (va_arg (va, alignedint) != i)
> +      abort ();
> +  va_end (va);
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  foo (5, 4, 3, 2, 1, 0);
> +  return 0;
> +}
> 

Re: [PATCH 2/2][ARM] fix movdi expander to avoid illegal ldrd/strd

[Richard Earnshaw]

On 03/07/15 16:27, Alan Lawrence wrote:
> The previous patch caused a regression in
> gcc.c-torture/execute/20040709-1.c at -O0 (only), and the new
> align_rec2.c test fails, both outputting an illegal assembler
> instruction (ldrd on an odd-numbered reg) from output_move_double in
> arm.c. Most routes have checks against such an illegal instruction, but
> expanding a function call can directly name such impossible register
> (pairs), bypassing the normal checks.
> 
> gcc/ChangeLog:
> 
>     * config/arm/arm.md (movdi): Avoid odd-number ldrd/strd in ARM state.
> 

OK.

R.

> arm_overalign_2.patch
> 
> 
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 164ac13a26289bf755c89e78a8a5f751883c6039..c6718282d2555f8cf9a4e9111b1393e1f7704983 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -5415,6 +5415,42 @@
>        if (!REG_P (operands[0]))
>  	operands[1] = force_reg (DImode, operands[1]);
>      }
> +  if (REG_P (operands[0]) && REGNO (operands[0]) < FIRST_VIRTUAL_REGISTER
> +      && !HARD_REGNO_MODE_OK (REGNO (operands[0]), DImode))
> +    {
> +      /* Avoid LDRD's into an odd-numbered register pair in ARM state
> +	 when expanding function calls.  */
> +      gcc_assert (can_create_pseudo_p ());
> +      if (MEM_P (operands[1]) && MEM_VOLATILE_P (operands[1]))
> +	{
> +	  /* Perform load into legal reg pair first, then move.  */
> +	  rtx reg = gen_reg_rtx (DImode);
> +	  emit_insn (gen_movdi (reg, operands[1]));
> +	  operands[1] = reg;
> +	}
> +      emit_move_insn (gen_lowpart (SImode, operands[0]),
> +		      gen_lowpart (SImode, operands[1]));
> +      emit_move_insn (gen_highpart (SImode, operands[0]),
> +	      gen_highpart (SImode, operands[1]));
> +      DONE;
> +    }
> +  else if (REG_P (operands[1]) && REGNO (operands[1]) < FIRST_VIRTUAL_REGISTER
> +	   && !HARD_REGNO_MODE_OK (REGNO (operands[1]), DImode))
> +    {
> +      /* Avoid LDRD's into an odd-numbered register pair in ARM state
> +	 when expanding function prologue.  */
> +      gcc_assert (can_create_pseudo_p ());
> +      rtx split_dest = (MEM_P (operands[0]) && MEM_VOLATILE_P (operands[0]))
> +		       ? gen_reg_rtx (DImode)
> +		       : operands[0];
> +      emit_move_insn (gen_lowpart (SImode, split_dest),
> +		      gen_lowpart (SImode, operands[1]));
> +      emit_move_insn (gen_highpart (SImode, split_dest),
> +	      gen_highpart (SImode, operands[1]));
> +      if (split_dest != operands[0])
> +	emit_insn (gen_movdi (operands[0], split_dest));
> +      DONE;
> +    }
>    "
>  )
>  
> 

Re: [PATCH 0/2][trunk+5 backport][ARM] PR/65956 Implement AAPCS updates for alignment attribute

[Richard Biener]

On July 3, 2015 5:24:24 PM GMT+02:00, Alan Lawrence <alan.lawrence@arm.com> wrote:
>This patch series implements the changes/additions to the ARM ABI
>proposed at 
>https://gcc.gnu.org/ml/gcc/2015-07/msg00040.html .
>
>The first patch is the ABI update. This is an ABI-breaking change for
>any code 
>using __attribute__((aligned(...))) on a public interface (a case not
>previously 
>defined by the AAPCS).
>
>This causes a regression of gcc.c-torture/execute/20040709-1.c at -O0
>(only), 
>and the align_rec2.c fails, both due to a latent bug where we can emit
>strd/ldrd 
>on an odd-numbered register in ARM state. The second patch prevents
>such illegal 
>instructions and fixes both tests.
>
>On trunk, tested via bootstrap + check-gcc on arm-none-linux-gnueabihf 
>(cortex-a15+neon). Also cross-tested arm-none-eabi with a number of
>variants.
>
>On gcc-5-branch, patches rebase cleanly, tested via profiledbootstrap +
>
>check-gcc. (Yes, profiledbootstrap succeeds.)

Just FYI, the back port is OK to apply once the trunk side is approved.

Thanks,
Richard.

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Jakub Jelinek]

On Fri, Jul 03, 2015 at 04:26:02PM +0100, Alan Lawrence wrote:
> These include tests of structs, scalars, and vectors - only general-purpose
> registers are affected by the ABI rules for alignment, but we can restrict
> the vector test to use the base AAPCS.
> 
> Prior to this patch, align2.c, align3.c and align_rec1.c were failing (the
> latter showing an internal inconsistency, the first two merely that GCC did
> not obey the new ABI).
> 
> With this patch, the align_rec2.c fails, and also
> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a latent bug
> where we can emit strd/ldrd on an odd-numbered register in ARM state, fixed
> by the second patch.
> 
> gcc/ChangeLog:
> 
> 	* config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
> 	alignment attribute, exploring one level down for aggregates.

Can you please also add the testcase from
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00278.html
to your patch set?  Or I can commit it separately after it is approved
(if it is).

	Jakub

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Richard Biener]

On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:
>On 03/07/15 16:26, Alan Lawrence wrote:
>> These include tests of structs, scalars, and vectors - only
>> general-purpose registers are affected by the ABI rules for
>alignment,
>> but we can restrict the vector test to use the base AAPCS.
>> 
>> Prior to this patch, align2.c, align3.c and align_rec1.c were failing
>> (the latter showing an internal inconsistency, the first two merely
>that
>> GCC did not obey the new ABI).
>> 
>> With this patch, the align_rec2.c fails, and also
>> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a
>latent
>> bug where we can emit strd/ldrd on an odd-numbered register in ARM
>> state, fixed by the second patch.
>> 
>> gcc/ChangeLog:
>> 
>>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>>     alignment attribute, exploring one level down for aggregates.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>     * gcc.target/arm/aapcs/align1.c: New.
>>     * gcc.target/arm/aapcs/align_rec1.c: New.
>>     * gcc.target/arm/aapcs/align2.c: New.
>>     * gcc.target/arm/aapcs/align_rec2.c: New.
>>     * gcc.target/arm/aapcs/align3.c: New.
>>     * gcc.target/arm/aapcs/align_rec3.c: New.
>>     * gcc.target/arm/aapcs/align4.c: New.
>>     * gcc.target/arm/aapcs/align_rec4.c: New.
>>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>>     * gcc.target/arm/aapcs/align_vararg2.c: New.
>> 
>> arm_overalign_1.patch
>> 
>> 
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index
>04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581
>100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS
>*pcum, tree fntype,
>>  static bool
>>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>  {
>> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>> +  if (!type)
>> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
>> +
>> +  if (!AGGREGATE_TYPE_P (type))
>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>> +
>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
>(field))
>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
>> +      return true;
>> +

Is this behavior correct for unions or aggregates with record or union members?

>
>Technically this is incorrect since AGGREGATE_TYPE_P includes
>ARRAY_TYPE
>and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>case though (unless there's a language that allows passing arrays by
>value).
>
>For array types I think you need to check TYPE_ALIGN (TREE_TYPE
>(type)).
>
>R.
>
>> +  return false;
>>  }
>>  
>>  
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>> @@ -0,0 +1,29 @@
>> +/* Test AAPCS layout (alignment).  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O" } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align1.c"
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +alignedint a = 11;
>> +alignedint b = 13;
>> +alignedint c = 17;
>> +alignedint d = 19;
>> +alignedint e = 23;
>> +alignedint f = 29;
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (alignedint, a, R0)
>> +  /* Attribute suggests R2, but we should use only natural
>alignment:  */
>> +  ARG (alignedint, b, R1)
>> +  ARG (alignedint, c, R2)
>> +  ARG (alignedint, d, R3)
>> +  ARG (alignedint, e, STACK)
>> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
>> +  LAST_ARG (alignedint, f, STACK + 4)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>> @@ -0,0 +1,30 @@
>> +/* Test AAPCS layout (alignment).  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O" } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align2.c"
>> +
>> +/* The underlying struct here has alignment 4.  */
>> +typedef struct __attribute__((aligned (8)))
>> +  {
>> +    int x;
>> +    int y;
>> +  } overaligned;
>> +
>> +/* A couple of instances, at 8-byte-aligned memory locations.  */
>> +overaligned a = { 2, 3 };
>> +overaligned b = { 5, 8 };
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (int, 7, R0)
>> +  /* Alignment should be 4.  */
>> +  ARG (overaligned, a, R1)
>> +  ARG (int, 9, R3)
>> +  ARG (int, 10, STACK)
>> +  /* Alignment should be 4.  */
>> +  LAST_ARG (overaligned, b, STACK + 4)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>> @@ -0,0 +1,42 @@
>> +/* Test AAPCS layout (alignment).  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O3" } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align3.c"
>> +
>> +/* Struct will be aligned to 8.  */
>> +struct s
>> +  {
>> +    int x;
>> +    /* 4 bytes padding here.  */
>> +    __attribute__((aligned (8))) int y;
>> +    /* 4 bytes padding here.  */
>> +  };
>> +
>> +typedef struct s __attribute__((aligned (4))) underaligned;
>> +
>> +#define EXPECTED_STRUCT_SIZE 16
>> +extern void link_failure (void);
>> +int
>> +foo ()
>> +{
>> +  /* Optimization gets rid of this before linking.  */
>> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
>> +    link_failure ();
>> +}
>> +
>> +underaligned a = { 1, 4 };
>> +underaligned b = { 9, 16 };
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (int, 3, R0)
>> +  /* Object alignment is 8, so split between 2 regs and 8 on stack. 
>*/
>> +  ARG (underaligned, a, R2)
>> +  ARG (int, 6, STACK + 8)
>> +  /* Object alignment is 8, so skip over STACK + 12.  */
>> +  LAST_ARG (underaligned, b, STACK + 16)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>> @@ -0,0 +1,29 @@
>> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-require-effective-target arm_neon_ok  } */
>> +/* { dg-options "-O" } */
>> +/* { dg-add-options arm_neon } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align4.c"
>> +
>> +#define PCSATTR __attribute__((pcs("aapcs")))
>> +
>> +#include <arm_neon.h>
>> +
>> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
>> +
>> +unalignedvec a = {11, 13};
>> +unalignedvec b = {17, 19};
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (int, 2, R0)
>> +  /* Attribute suggests R1, but we should use natural alignment:  */
>> +  ARG (unalignedvec, a, R2)
>> +  ARG (int, 6, STACK)
>> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
>> +  LAST_ARG (unalignedvec, b, STACK + 8)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>> @@ -0,0 +1,36 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +alignedint a = 11;
>> +alignedint b = 13;
>> +alignedint c = 17;
>> +alignedint d = 19;
>> +alignedint e = 23;
>> +alignedint f = 29;
>> +
>> +void
>> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
>> +     alignedint stack, alignedint stack4)
>> +{
>> +  if (r0 != a
>> +      || r1 != b
>> +      || r2 != c
>> +      || r3 != d
>> +      || stack != e
>> +      || stack4 !=f)
>> +    abort ();
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  foo (a, b, c, d, e, f);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>> @@ -0,0 +1,41 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>> +extern void abort (void);
>> +
>> +typedef struct __attribute__((aligned (8)))
>> +  {
>> +    int x;
>> +    int y;
>> +  } overaligned;
>> +
>> +overaligned a = { 2, 3 };
>> +overaligned b = { 5, 8 };
>> +
>> +void
>> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
>> +{
>> +  if (r0 != 7 || r3 != 9 || stack != 10)
>> +    abort ();
>> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
>> +    abort ();
>> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
>> +    abort ();
>> +  int addr = ((int) &stack4) & 7;
>> +  if (addr != 0)
>> +    {
>> +      __builtin_printf ("Alignment was %d\n", addr);
>> +      abort ();
>> +    }
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  f (7, a, 9, 10, b);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>> @@ -0,0 +1,43 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>> +extern void abort (void);
>> +
>> +
>> +/* Struct will be aligned to 8.  */
>> +struct s
>> +  {
>> +    int x;
>> +    /* 4 bytes padding here.  */
>> +    __attribute__((aligned (8))) int y;
>> +    /* 4 bytes padding here.  */
>> +  };
>> +
>> +typedef struct s __attribute__((aligned (4))) underaligned;
>> +
>> +underaligned a = { 1, 4 };
>> +underaligned b = { 9, 16 };
>> +
>> +void
>> +f (int r0, underaligned r2, int stack8, underaligned stack16)
>> +{
>> +  if (r0 != 3 || stack8 != 6)
>> +    abort ();
>> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
>> +    abort ();
>> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
>> +    abort ();
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  f (3, a, 6, b);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>> @@ -0,0 +1,33 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-require-effective-target arm_neon_ok } */
>> +/* { dg-options "-O -fno-inline" } */
>> +/* { dg-add-options arm_neon } */
>> +
>> +#include <arm_neon.h>
>> +
>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
>> +
>> +unalignedvec a = {11, 13};
>> +unalignedvec b = {17, 19};
>> +
>> +void
>> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
>> +{
>> +  if (r0 != 2 || s0 != 6
>> +      || memcmp ( (void *) &r2, (void *) &a, 16)
>> +      || memcmp ( (void *) &s8, (void *) &b, 16))
>> +    abort ();
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  foo (2, a, 6, b);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>> @@ -0,0 +1,36 @@
>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +#include <stdarg.h>
>> +
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +void
>> +foo (int i, ...)
>> +{
>> +  va_list va;
>> +  va_start (va, i);
>> +  /* Arguments should be passed in the same registers as if they
>were ints.  */
>> +  while (i-- > 0)
>> +    if (va_arg (va, int) != i)
>> +      abort ();
>> +  va_end (va);
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  alignedint a = 5;
>> +  alignedint b = 4;
>> +  alignedint c = 3;
>> +  alignedint d = 2;
>> +  alignedint e = 1;
>> +  alignedint f = 0;
>> +  foo (a, b, c, d, e, f);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>> @@ -0,0 +1,30 @@
>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +#include <stdarg.h>
>> +
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +void
>> +foo (int i, ...)
>> +{
>> +  va_list va;
>> +  va_start (va, i);
>> +  /* alignedint should be pulled out of regs/stack just like an int.
> */
>> +  while (i-- > 0)
>> +    if (va_arg (va, alignedint) != i)
>> +      abort ();
>> +  va_end (va);
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  foo (5, 4, 3, 2, 1, 0);
>> +  return 0;
>> +}
>> 

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Richard Earnshaw]

On 03/07/15 19:24, Richard Biener wrote:
> On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:
>> On 03/07/15 16:26, Alan Lawrence wrote:
>>> These include tests of structs, scalars, and vectors - only
>>> general-purpose registers are affected by the ABI rules for
>> alignment,
>>> but we can restrict the vector test to use the base AAPCS.
>>>
>>> Prior to this patch, align2.c, align3.c and align_rec1.c were failing
>>> (the latter showing an internal inconsistency, the first two merely
>> that
>>> GCC did not obey the new ABI).
>>>
>>> With this patch, the align_rec2.c fails, and also
>>> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a
>> latent
>>> bug where we can emit strd/ldrd on an odd-numbered register in ARM
>>> state, fixed by the second patch.
>>>
>>> gcc/ChangeLog:
>>>
>>>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>>>     alignment attribute, exploring one level down for aggregates.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/arm/aapcs/align1.c: New.
>>>     * gcc.target/arm/aapcs/align_rec1.c: New.
>>>     * gcc.target/arm/aapcs/align2.c: New.
>>>     * gcc.target/arm/aapcs/align_rec2.c: New.
>>>     * gcc.target/arm/aapcs/align3.c: New.
>>>     * gcc.target/arm/aapcs/align_rec3.c: New.
>>>     * gcc.target/arm/aapcs/align4.c: New.
>>>     * gcc.target/arm/aapcs/align_rec4.c: New.
>>>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>>>     * gcc.target/arm/aapcs/align_vararg2.c: New.
>>>
>>> arm_overalign_1.patch
>>>
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index
>> 04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581
>> 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS
>> *pcum, tree fntype,
>>>  static bool
>>>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>>  {
>>> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>>> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>>> +  if (!type)
>>> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
>>> +
>>> +  if (!AGGREGATE_TYPE_P (type))
>>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>>> +
>>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
>> (field))
>>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
>>> +      return true;
>>> +
> 
> Is this behavior correct for unions or aggregates with record or union members?

Yes, at least that was my intention.  It's an error in the wording of
the proposed change, which I think should say "composite types" not
"aggregate types".

R.

> 
>>
>> Technically this is incorrect since AGGREGATE_TYPE_P includes
>> ARRAY_TYPE
>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>> case though (unless there's a language that allows passing arrays by
>> value).
>>
>> For array types I think you need to check TYPE_ALIGN (TREE_TYPE
>> (type)).
>>
>> R.
>>
>>> +  return false;
>>>  }
>>>  
>>>  
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>> @@ -0,0 +1,29 @@
>>> +/* Test AAPCS layout (alignment).  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O" } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align1.c"
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +alignedint a = 11;
>>> +alignedint b = 13;
>>> +alignedint c = 17;
>>> +alignedint d = 19;
>>> +alignedint e = 23;
>>> +alignedint f = 29;
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (alignedint, a, R0)
>>> +  /* Attribute suggests R2, but we should use only natural
>> alignment:  */
>>> +  ARG (alignedint, b, R1)
>>> +  ARG (alignedint, c, R2)
>>> +  ARG (alignedint, d, R3)
>>> +  ARG (alignedint, e, STACK)
>>> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
>>> +  LAST_ARG (alignedint, f, STACK + 4)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>> @@ -0,0 +1,30 @@
>>> +/* Test AAPCS layout (alignment).  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O" } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align2.c"
>>> +
>>> +/* The underlying struct here has alignment 4.  */
>>> +typedef struct __attribute__((aligned (8)))
>>> +  {
>>> +    int x;
>>> +    int y;
>>> +  } overaligned;
>>> +
>>> +/* A couple of instances, at 8-byte-aligned memory locations.  */
>>> +overaligned a = { 2, 3 };
>>> +overaligned b = { 5, 8 };
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (int, 7, R0)
>>> +  /* Alignment should be 4.  */
>>> +  ARG (overaligned, a, R1)
>>> +  ARG (int, 9, R3)
>>> +  ARG (int, 10, STACK)
>>> +  /* Alignment should be 4.  */
>>> +  LAST_ARG (overaligned, b, STACK + 4)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>> @@ -0,0 +1,42 @@
>>> +/* Test AAPCS layout (alignment).  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O3" } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align3.c"
>>> +
>>> +/* Struct will be aligned to 8.  */
>>> +struct s
>>> +  {
>>> +    int x;
>>> +    /* 4 bytes padding here.  */
>>> +    __attribute__((aligned (8))) int y;
>>> +    /* 4 bytes padding here.  */
>>> +  };
>>> +
>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>> +
>>> +#define EXPECTED_STRUCT_SIZE 16
>>> +extern void link_failure (void);
>>> +int
>>> +foo ()
>>> +{
>>> +  /* Optimization gets rid of this before linking.  */
>>> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
>>> +    link_failure ();
>>> +}
>>> +
>>> +underaligned a = { 1, 4 };
>>> +underaligned b = { 9, 16 };
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (int, 3, R0)
>>> +  /* Object alignment is 8, so split between 2 regs and 8 on stack. 
>> */
>>> +  ARG (underaligned, a, R2)
>>> +  ARG (int, 6, STACK + 8)
>>> +  /* Object alignment is 8, so skip over STACK + 12.  */
>>> +  LAST_ARG (underaligned, b, STACK + 16)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>> @@ -0,0 +1,29 @@
>>> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-require-effective-target arm_neon_ok  } */
>>> +/* { dg-options "-O" } */
>>> +/* { dg-add-options arm_neon } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align4.c"
>>> +
>>> +#define PCSATTR __attribute__((pcs("aapcs")))
>>> +
>>> +#include <arm_neon.h>
>>> +
>>> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
>>> +
>>> +unalignedvec a = {11, 13};
>>> +unalignedvec b = {17, 19};
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (int, 2, R0)
>>> +  /* Attribute suggests R1, but we should use natural alignment:  */
>>> +  ARG (unalignedvec, a, R2)
>>> +  ARG (int, 6, STACK)
>>> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
>>> +  LAST_ARG (unalignedvec, b, STACK + 8)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>> @@ -0,0 +1,36 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +alignedint a = 11;
>>> +alignedint b = 13;
>>> +alignedint c = 17;
>>> +alignedint d = 19;
>>> +alignedint e = 23;
>>> +alignedint f = 29;
>>> +
>>> +void
>>> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
>>> +     alignedint stack, alignedint stack4)
>>> +{
>>> +  if (r0 != a
>>> +      || r1 != b
>>> +      || r2 != c
>>> +      || r3 != d
>>> +      || stack != e
>>> +      || stack4 !=f)
>>> +    abort ();
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  foo (a, b, c, d, e, f);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>> @@ -0,0 +1,41 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>>> +extern void abort (void);
>>> +
>>> +typedef struct __attribute__((aligned (8)))
>>> +  {
>>> +    int x;
>>> +    int y;
>>> +  } overaligned;
>>> +
>>> +overaligned a = { 2, 3 };
>>> +overaligned b = { 5, 8 };
>>> +
>>> +void
>>> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
>>> +{
>>> +  if (r0 != 7 || r3 != 9 || stack != 10)
>>> +    abort ();
>>> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
>>> +    abort ();
>>> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
>>> +    abort ();
>>> +  int addr = ((int) &stack4) & 7;
>>> +  if (addr != 0)
>>> +    {
>>> +      __builtin_printf ("Alignment was %d\n", addr);
>>> +      abort ();
>>> +    }
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  f (7, a, 9, 10, b);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>> @@ -0,0 +1,43 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>>> +extern void abort (void);
>>> +
>>> +
>>> +/* Struct will be aligned to 8.  */
>>> +struct s
>>> +  {
>>> +    int x;
>>> +    /* 4 bytes padding here.  */
>>> +    __attribute__((aligned (8))) int y;
>>> +    /* 4 bytes padding here.  */
>>> +  };
>>> +
>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>> +
>>> +underaligned a = { 1, 4 };
>>> +underaligned b = { 9, 16 };
>>> +
>>> +void
>>> +f (int r0, underaligned r2, int stack8, underaligned stack16)
>>> +{
>>> +  if (r0 != 3 || stack8 != 6)
>>> +    abort ();
>>> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
>>> +    abort ();
>>> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
>>> +    abort ();
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  f (3, a, 6, b);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>> @@ -0,0 +1,33 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-require-effective-target arm_neon_ok } */
>>> +/* { dg-options "-O -fno-inline" } */
>>> +/* { dg-add-options arm_neon } */
>>> +
>>> +#include <arm_neon.h>
>>> +
>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
>>> +
>>> +unalignedvec a = {11, 13};
>>> +unalignedvec b = {17, 19};
>>> +
>>> +void
>>> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
>>> +{
>>> +  if (r0 != 2 || s0 != 6
>>> +      || memcmp ( (void *) &r2, (void *) &a, 16)
>>> +      || memcmp ( (void *) &s8, (void *) &b, 16))
>>> +    abort ();
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  foo (2, a, 6, b);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>> @@ -0,0 +1,36 @@
>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +#include <stdarg.h>
>>> +
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +void
>>> +foo (int i, ...)
>>> +{
>>> +  va_list va;
>>> +  va_start (va, i);
>>> +  /* Arguments should be passed in the same registers as if they
>> were ints.  */
>>> +  while (i-- > 0)
>>> +    if (va_arg (va, int) != i)
>>> +      abort ();
>>> +  va_end (va);
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  alignedint a = 5;
>>> +  alignedint b = 4;
>>> +  alignedint c = 3;
>>> +  alignedint d = 2;
>>> +  alignedint e = 1;
>>> +  alignedint f = 0;
>>> +  foo (a, b, c, d, e, f);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>> @@ -0,0 +1,30 @@
>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +#include <stdarg.h>
>>> +
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +void
>>> +foo (int i, ...)
>>> +{
>>> +  va_list va;
>>> +  va_start (va, i);
>>> +  /* alignedint should be pulled out of regs/stack just like an int.
>> */
>>> +  while (i-- > 0)
>>> +    if (va_arg (va, alignedint) != i)
>>> +      abort ();
>>> +  va_end (va);
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  foo (5, 4, 3, 2, 1, 0);
>>> +  return 0;
>>> +}
>>>
> 
> 

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Richard Biener]

On July 3, 2015 10:43:30 PM GMT+02:00, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:
>On 03/07/15 19:24, Richard Biener wrote:
>> On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw
><Richard.Earnshaw@foss.arm.com> wrote:
>>> On 03/07/15 16:26, Alan Lawrence wrote:
>>>> These include tests of structs, scalars, and vectors - only
>>>> general-purpose registers are affected by the ABI rules for
>>> alignment,
>>>> but we can restrict the vector test to use the base AAPCS.
>>>>
>>>> Prior to this patch, align2.c, align3.c and align_rec1.c were
>failing
>>>> (the latter showing an internal inconsistency, the first two merely
>>> that
>>>> GCC did not obey the new ABI).
>>>>
>>>> With this patch, the align_rec2.c fails, and also
>>>> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a
>>> latent
>>>> bug where we can emit strd/ldrd on an odd-numbered register in ARM
>>>> state, fixed by the second patch.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>>>>     alignment attribute, exploring one level down for aggregates.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * gcc.target/arm/aapcs/align1.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec1.c: New.
>>>>     * gcc.target/arm/aapcs/align2.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec2.c: New.
>>>>     * gcc.target/arm/aapcs/align3.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec3.c: New.
>>>>     * gcc.target/arm/aapcs/align4.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec4.c: New.
>>>>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>>>>     * gcc.target/arm/aapcs/align_vararg2.c: New.
>>>>
>>>> arm_overalign_1.patch
>>>>
>>>>
>>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>>> index
>>>
>04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581
>>> 100644
>>>> --- a/gcc/config/arm/arm.c
>>>> +++ b/gcc/config/arm/arm.c
>>>> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS
>>> *pcum, tree fntype,
>>>>  static bool
>>>>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>>>  {
>>>> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>>>> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>>>> +  if (!type)
>>>> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
>>>> +
>>>> +  if (!AGGREGATE_TYPE_P (type))
>>>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>>>> +
>>>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
>>> (field))
>>>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
>>>> +      return true;

I also believe this loop is equivalent to checking TYPE_ALIGN of the aggregate type?

I'll double check your wording in the abi document, but it seems to be unclear whether packed and not packed structs should be passed the same (considering layout differences).  OTOH the above function is only relevant for register passing? (Likewise the abi document changes?)

>> 
>> Is this behavior correct for unions or aggregates with record or
>union members?
>
>Yes, at least that was my intention.  It's an error in the wording of
>the proposed change, which I think should say "composite types" not
>"aggregate types".
>
>R.
>
>> 
>>>
>>> Technically this is incorrect since AGGREGATE_TYPE_P includes
>>> ARRAY_TYPE
>>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach
>that
>>> case though (unless there's a language that allows passing arrays by
>>> value).
>>>
>>> For array types I think you need to check TYPE_ALIGN (TREE_TYPE
>>> (type)).
>>>
>>> R.
>>>
>>>> +  return false;
>>>>  }
>>>>  
>>>>  
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* Test AAPCS layout (alignment).  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O" } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align1.c"
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +alignedint a = 11;
>>>> +alignedint b = 13;
>>>> +alignedint c = 17;
>>>> +alignedint d = 19;
>>>> +alignedint e = 23;
>>>> +alignedint f = 29;
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (alignedint, a, R0)
>>>> +  /* Attribute suggests R2, but we should use only natural
>>> alignment:  */
>>>> +  ARG (alignedint, b, R1)
>>>> +  ARG (alignedint, c, R2)
>>>> +  ARG (alignedint, d, R3)
>>>> +  ARG (alignedint, e, STACK)
>>>> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
>>>> +  LAST_ARG (alignedint, f, STACK + 4)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>>> @@ -0,0 +1,30 @@
>>>> +/* Test AAPCS layout (alignment).  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O" } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align2.c"
>>>> +
>>>> +/* The underlying struct here has alignment 4.  */
>>>> +typedef struct __attribute__((aligned (8)))
>>>> +  {
>>>> +    int x;
>>>> +    int y;
>>>> +  } overaligned;
>>>> +
>>>> +/* A couple of instances, at 8-byte-aligned memory locations.  */
>>>> +overaligned a = { 2, 3 };
>>>> +overaligned b = { 5, 8 };
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (int, 7, R0)
>>>> +  /* Alignment should be 4.  */
>>>> +  ARG (overaligned, a, R1)
>>>> +  ARG (int, 9, R3)
>>>> +  ARG (int, 10, STACK)
>>>> +  /* Alignment should be 4.  */
>>>> +  LAST_ARG (overaligned, b, STACK + 4)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>>> @@ -0,0 +1,42 @@
>>>> +/* Test AAPCS layout (alignment).  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O3" } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align3.c"
>>>> +
>>>> +/* Struct will be aligned to 8.  */
>>>> +struct s
>>>> +  {
>>>> +    int x;
>>>> +    /* 4 bytes padding here.  */
>>>> +    __attribute__((aligned (8))) int y;
>>>> +    /* 4 bytes padding here.  */
>>>> +  };
>>>> +
>>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>>> +
>>>> +#define EXPECTED_STRUCT_SIZE 16
>>>> +extern void link_failure (void);
>>>> +int
>>>> +foo ()
>>>> +{
>>>> +  /* Optimization gets rid of this before linking.  */
>>>> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
>>>> +    link_failure ();
>>>> +}
>>>> +
>>>> +underaligned a = { 1, 4 };
>>>> +underaligned b = { 9, 16 };
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (int, 3, R0)
>>>> +  /* Object alignment is 8, so split between 2 regs and 8 on
>stack. 
>>> */
>>>> +  ARG (underaligned, a, R2)
>>>> +  ARG (int, 6, STACK + 8)
>>>> +  /* Object alignment is 8, so skip over STACK + 12.  */
>>>> +  LAST_ARG (underaligned, b, STACK + 16)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-require-effective-target arm_neon_ok  } */
>>>> +/* { dg-options "-O" } */
>>>> +/* { dg-add-options arm_neon } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align4.c"
>>>> +
>>>> +#define PCSATTR __attribute__((pcs("aapcs")))
>>>> +
>>>> +#include <arm_neon.h>
>>>> +
>>>> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
>>>> +
>>>> +unalignedvec a = {11, 13};
>>>> +unalignedvec b = {17, 19};
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (int, 2, R0)
>>>> +  /* Attribute suggests R1, but we should use natural alignment: 
>*/
>>>> +  ARG (unalignedvec, a, R2)
>>>> +  ARG (int, 6, STACK)
>>>> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
>>>> +  LAST_ARG (unalignedvec, b, STACK + 8)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>>> @@ -0,0 +1,36 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +alignedint a = 11;
>>>> +alignedint b = 13;
>>>> +alignedint c = 17;
>>>> +alignedint d = 19;
>>>> +alignedint e = 23;
>>>> +alignedint f = 29;
>>>> +
>>>> +void
>>>> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
>>>> +     alignedint stack, alignedint stack4)
>>>> +{
>>>> +  if (r0 != a
>>>> +      || r1 != b
>>>> +      || r2 != c
>>>> +      || r3 != d
>>>> +      || stack != e
>>>> +      || stack4 !=f)
>>>> +    abort ();
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  foo (a, b, c, d, e, f);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>>> @@ -0,0 +1,41 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__
>n);
>>>> +extern void abort (void);
>>>> +
>>>> +typedef struct __attribute__((aligned (8)))
>>>> +  {
>>>> +    int x;
>>>> +    int y;
>>>> +  } overaligned;
>>>> +
>>>> +overaligned a = { 2, 3 };
>>>> +overaligned b = { 5, 8 };
>>>> +
>>>> +void
>>>> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
>>>> +{
>>>> +  if (r0 != 7 || r3 != 9 || stack != 10)
>>>> +    abort ();
>>>> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
>>>> +    abort ();
>>>> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
>>>> +    abort ();
>>>> +  int addr = ((int) &stack4) & 7;
>>>> +  if (addr != 0)
>>>> +    {
>>>> +      __builtin_printf ("Alignment was %d\n", addr);
>>>> +      abort ();
>>>> +    }
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  f (7, a, 9, 10, b);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>>> @@ -0,0 +1,43 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__
>n);
>>>> +extern void abort (void);
>>>> +
>>>> +
>>>> +/* Struct will be aligned to 8.  */
>>>> +struct s
>>>> +  {
>>>> +    int x;
>>>> +    /* 4 bytes padding here.  */
>>>> +    __attribute__((aligned (8))) int y;
>>>> +    /* 4 bytes padding here.  */
>>>> +  };
>>>> +
>>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>>> +
>>>> +underaligned a = { 1, 4 };
>>>> +underaligned b = { 9, 16 };
>>>> +
>>>> +void
>>>> +f (int r0, underaligned r2, int stack8, underaligned stack16)
>>>> +{
>>>> +  if (r0 != 3 || stack8 != 6)
>>>> +    abort ();
>>>> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
>>>> +    abort ();
>>>> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof
>(underaligned)))
>>>> +    abort ();
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  f (3, a, 6, b);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>>> @@ -0,0 +1,33 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-require-effective-target arm_neon_ok } */
>>>> +/* { dg-options "-O -fno-inline" } */
>>>> +/* { dg-add-options arm_neon } */
>>>> +
>>>> +#include <arm_neon.h>
>>>> +
>>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__
>n);
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
>>>> +
>>>> +unalignedvec a = {11, 13};
>>>> +unalignedvec b = {17, 19};
>>>> +
>>>> +void
>>>> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
>>>> +{
>>>> +  if (r0 != 2 || s0 != 6
>>>> +      || memcmp ( (void *) &r2, (void *) &a, 16)
>>>> +      || memcmp ( (void *) &s8, (void *) &b, 16))
>>>> +    abort ();
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  foo (2, a, 6, b);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>>> @@ -0,0 +1,36 @@
>>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +#include <stdarg.h>
>>>> +
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +void
>>>> +foo (int i, ...)
>>>> +{
>>>> +  va_list va;
>>>> +  va_start (va, i);
>>>> +  /* Arguments should be passed in the same registers as if they
>>> were ints.  */
>>>> +  while (i-- > 0)
>>>> +    if (va_arg (va, int) != i)
>>>> +      abort ();
>>>> +  va_end (va);
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  alignedint a = 5;
>>>> +  alignedint b = 4;
>>>> +  alignedint c = 3;
>>>> +  alignedint d = 2;
>>>> +  alignedint e = 1;
>>>> +  alignedint f = 0;
>>>> +  foo (a, b, c, d, e, f);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>>> @@ -0,0 +1,30 @@
>>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +#include <stdarg.h>
>>>> +
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +void
>>>> +foo (int i, ...)
>>>> +{
>>>> +  va_list va;
>>>> +  va_start (va, i);
>>>> +  /* alignedint should be pulled out of regs/stack just like an
>int.
>>> */
>>>> +  while (i-- > 0)
>>>> +    if (va_arg (va, alignedint) != i)
>>>> +      abort ();
>>>> +  va_end (va);
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  foo (5, 4, 3, 2, 1, 0);
>>>> +  return 0;
>>>> +}
>>>>
>> 
>> 

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Jakub Jelinek]

On Sat, Jul 04, 2015 at 12:57:36PM +0200, Richard Biener wrote:
> >>>> +  if (!AGGREGATE_TYPE_P (type))
> >>>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
> >>>> +
> >>>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
> >>> (field))
> >>>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
> >>>> +      return true;
> 
> I also believe this loop is equivalent to checking TYPE_ALIGN of the aggregate type?

Is it?

What if you do
struct __attribute__((aligned (32))) S { char a; int b; char c; }; ?
In this case, TYPE_MAIN_VARIANT of S is S itself, and has TYPE_USER_ALIGN
and TYPE_ALIGN 256.

	Jakub

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Eric Botcazou]

> Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
> case though (unless there's a language that allows passing arrays by value).

Ada passes small array types by the method specified by the pass_by_reference 
hook (and large array types by reference).

-- 
Eric Botcazou

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

Richard Biener wrote:
> 
> I also believe this loop is equivalent to checking TYPE_ALIGN of the aggregate type?

Jakub is correct: the intention is to discard any top-level alignment attribute 
on a struct declaration.

> I'll double check your wording in the abi document, but it seems to be unclear whether packed and not packed structs should be passed the same (considering layout differences).  OTOH the above function is only relevant for register passing? (Likewise the abi document changes?)

It also affects the alignment of things passed on the stack. 'Packed' structs 
are affected too: the outer 'packed' will have no effect on the position on the 
stack / in registers, as you say; layout will still be packed.

>>> Is this behavior correct for unions or aggregates with record or
>> union members?

To clarify Richard Earnshaw's statement: The intention is that 'member 
alignment' is pretty much gcc's TYPE_ALIGN (actually the source code type 
declaration - which is the same for for struct members, but ignoring cases where 
other opts like SRA figure out a larger TYPE_ALIGN). 'Natural alignment' is not 
directly available in GCC under all circumstances, hence having to compute it here.

--Alan

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

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

Eric Botcazou wrote:
>> Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>> case though (unless there's a language that allows passing arrays by value).
> 
> Ada passes small array types by the method specified by the pass_by_reference 
> hook (and large array types by reference).

Ok, thanks. Here's a revised patch that handles array types. Again I've tested 
on both trunk (bootstrap + check-gcc) and gcc-5-branch (profiledbootstrap now 
succeeding + check-gcc). Jakub's pr65956.c testcase also now passes.

The new code lacks a testcase; from what Eric says, it's possible we can write 
one using Ada, but I don't know any Ada myself, so I think any testcase should 
follow in a separate patch.

Neither have I managed to run a check-ada yet, as I don't presently have a 
working Ada compiler with which to bootstrap gcc's Ada frontend. Working on this 
now.

--Alan

gcc/ChangeLog:

	* config/arm/arm.c (arm_needs_doubleword_align) : Drop any outer
	alignment attribute, exploring one level down for records and arrays.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm_overalign_1v2.patch --]
[-- Type: text/x-patch; name=arm_overalign_1v2.patch, Size: 1320 bytes --]

commit f8bd310d65f2b8fd8d7e1151a4a1f84489738029
Author: Alan Lawrence <alan.lawrence@arm.com>
Date:   Wed Jun 3 18:22:36 2015 +0100

    arm_needs_doubleword_align: explore one level for aggregates, also arrays.

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e79a369..e12198a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6151,8 +6151,23 @@ arm_init_cumulative_args (CUMULATIVE_ARGS *pcum, tree fntype,
 static bool
 arm_needs_doubleword_align (machine_mode mode, const_tree type)
 {
-  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
-	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
+  if (!type)
+    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
+
+  /* Scalar and vector types: Use natural alignment, i.e. of base type.  */
+  if (!AGGREGATE_TYPE_P (type))
+    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
+
+  /* Array types: Use member alignment of element type.  */
+  if (TREE_CODE (type) == ARRAY_TYPE)
+    return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY;
+
+  /* Record/aggregate types: Use greatest member alignment of any member.  */ 
+  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
+    if (DECL_ALIGN (field) > PARM_BOUNDARY)
+      return true;
+
+  return false;
 }
 
 

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Ramana Radhakrishnan]

On 06/07/15 12:00, Alan Lawrence wrote:
> Eric Botcazou wrote:
>>> Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
>>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>>> case though (unless there's a language that allows passing arrays by value).
>>
>> Ada passes small array types by the method specified by the pass_by_reference hook (and large array types by reference).
> 
> Ok, thanks. Here's a revised patch that handles array types. Again I've tested on both trunk (bootstrap + check-gcc) and gcc-5-branch (profiledbootstrap now succeeding + check-gcc). Jakub's pr65956.c testcase also now passes.
> 

> The new code lacks a testcase; from what Eric says, it's possible we can write one using Ada, but I don't know any Ada myself, so I think any testcase should follow in a separate patch.
> 
> Neither have I managed to run a check-ada yet, as I don't presently have a working Ada compiler with which to bootstrap gcc's Ada frontend. Working on this now.

This is OK, the ada testing can go in parallel and we should take this in to not delay rc1 any further.



regards
Ramana

> 
> --Alan
> 
> gcc/ChangeLog:
> 
>     * config/arm/arm.c (arm_needs_doubleword_align) : Drop any outer
>     alignment attribute, exploring one level down for records and arrays.

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

Trying to push these now (svn!), patch 2 is going first.

I realize my second iteration of patch 1/2, dropped the testcases from the first 
version. Okay to include those as per 
https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00215.html ?

Cheers, Alan

Ramana Radhakrishnan wrote:
> 
> On 06/07/15 12:00, Alan Lawrence wrote:
>> Eric Botcazou wrote:
>>>> Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
>>>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>>>> case though (unless there's a language that allows passing arrays by value).
>>> Ada passes small array types by the method specified by the pass_by_reference hook (and large array types by reference).
>> Ok, thanks. Here's a revised patch that handles array types. Again I've tested on both trunk (bootstrap + check-gcc) and gcc-5-branch (profiledbootstrap now succeeding + check-gcc). Jakub's pr65956.c testcase also now passes.
>>
> 
>> The new code lacks a testcase; from what Eric says, it's possible we can write one using Ada, but I don't know any Ada myself, so I think any testcase should follow in a separate patch.
>>
>> Neither have I managed to run a check-ada yet, as I don't presently have a working Ada compiler with which to bootstrap gcc's Ada frontend. Working on this now.
> 
> This is OK, the ada testing can go in parallel and we should take this in to not delay rc1 any further.
> 
> 
> 
> regards
> Ramana
> 
>> --Alan
>>
>> gcc/ChangeLog:
>>
>>     * config/arm/arm.c (arm_needs_doubleword_align) : Drop any outer
>>     alignment attribute, exploring one level down for records and arrays.
> 

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Ramana Radhakrishnan]

On 06/07/15 17:38, Alan Lawrence wrote:
> Trying to push these now (svn!), patch 2 is going first.
> 
> I realize my second iteration of patch 1/2, dropped the testcases from the first version. Okay to include those as per https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00215.html ?

Yeah the tests are fine to go in as long as the testcases showed no regressions ;) 

What about Jakub's tests ? Is he adding them in or are you considering them here ?

Ramana

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

Ramana Radhakrishnan wrote:
> 
> On 06/07/15 17:38, Alan Lawrence wrote:
>> Trying to push these now (svn!), patch 2 is going first.
>>
>> I realize my second iteration of patch 1/2, dropped the testcases from the first version. Okay to include those as per https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00215.html ?
> 
> Yeah the tests are fine to go in as long as the testcases showed no regressions ;) 
> 
> What about Jakub's tests ? Is he adding them in or are you considering them here ?
> 
> Ramana
> 

I'll add Jakub's test, but as a separate commit, I wouldn't want to claim 
authorship of that one ;-)

--Alan

Re: [PATCH 2/2][ARM] fix movdi expander to avoid illegal ldrd/strd

[Alan Lawrence]

Richard Earnshaw wrote:
> On 03/07/15 16:27, Alan Lawrence wrote:
>> The previous patch caused a regression in
>> gcc.c-torture/execute/20040709-1.c at -O0 (only), and the new
>> align_rec2.c test fails, both outputting an illegal assembler
>> instruction (ldrd on an odd-numbered reg) from output_move_double in
>> arm.c. Most routes have checks against such an illegal instruction, but
>> expanding a function call can directly name such impossible register
>> (pairs), bypassing the normal checks.
>>
>> gcc/ChangeLog:
>>
>>     * config/arm/arm.md (movdi): Avoid odd-number ldrd/strd in ARM state.
>>
> 
> OK.

Both patches, plus Jakub's test, pushed onto trunk (r221461/5/6), and 
gcc-5-branch (r225467/9/70), with an obvious comment fix to the movdi patch 
(LDRD's into, STRD's from), as below.

Cheers, Alan


Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md       (revision 225457)
+++ gcc/config/arm/arm.md       (working copy)
@@ -5481,6 +5481,42 @@
        if (!REG_P (operands[0]))
         operands[1] = force_reg (DImode, operands[1]);
      }
+  if (REG_P (operands[0]) && REGNO (operands[0]) < FIRST_VIRTUAL_REGISTER
+      && !HARD_REGNO_MODE_OK (REGNO (operands[0]), DImode))
+    {
+      /* Avoid LDRD's into an odd-numbered register pair in ARM state
+        when expanding function calls.  */
+      gcc_assert (can_create_pseudo_p ());
+      if (MEM_P (operands[1]) && MEM_VOLATILE_P (operands[1]))
+       {
+         /* Perform load into legal reg pair first, then move.  */
+         rtx reg = gen_reg_rtx (DImode);
+         emit_insn (gen_movdi (reg, operands[1]));
+         operands[1] = reg;
+       }
+      emit_move_insn (gen_lowpart (SImode, operands[0]),
+                     gen_lowpart (SImode, operands[1]));
+      emit_move_insn (gen_highpart (SImode, operands[0]),
+                     gen_highpart (SImode, operands[1]));
+      DONE;
+    }
+  else if (REG_P (operands[1]) && REGNO (operands[1]) < FIRST_VIRTUAL_REGISTER
+          && !HARD_REGNO_MODE_OK (REGNO (operands[1]), DImode))
+    {
+      /* Avoid STRD's from an odd-numbered register pair in ARM state
+        when expanding function prologue.  */
+      gcc_assert (can_create_pseudo_p ());
+      rtx split_dest = (MEM_P (operands[0]) && MEM_VOLATILE_P (operands[0]))
+                      ? gen_reg_rtx (DImode)
+                      : operands[0];
+      emit_move_insn (gen_lowpart (SImode, split_dest),
+                     gen_lowpart (SImode, operands[1]));
+      emit_move_insn (gen_highpart (SImode, split_dest),
+                     gen_highpart (SImode, operands[1]));
+      if (split_dest != operands[0])
+       emit_insn (gen_movdi (operands[0], split_dest));
+      DONE;
+    }
    "
  )

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

Ramana Radhakrishnan wrote:
> 
> This is OK, the ada testing can go in parallel and we should take this in to not delay rc1 any further.

I can confirm, no regressions in check-ada (gcc/testsuite/gnats and 
gcc/testsuite/acats) following an ada bootstrap on cortex-a15/neon/hard-float.

That's the existing tests - nothing specifically testing conformance to the 
AAPCS updates (wrt. arrays), of course.

Cheers, Alan

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Jakub Jelinek]

On Mon, Jul 06, 2015 at 05:38:35PM +0100, Alan Lawrence wrote:
> Trying to push these now (svn!), patch 2 is going first.
> 
> I realize my second iteration of patch 1/2, dropped the testcases from the
> first version. Okay to include those as per
> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00215.html ?

FYI, it seems that (most likely) the PR65956 changes on gcc-5-branch
broke libgnat ABI compatibility on arm - it seems that getsubs.adb
from macrosub proglet (and others) are during make check compiled/linked
with system gnatmake/gcc, but the program is run at runtime
against the new libgnat-5.so.  If I run it manually against
system libgnat, it works, otherwise it hangs, when Fill_Table from
getsubs.adb calls Get_Line, and indeed it looks like the argument passing
for Get_Line changed and on the callee side it thinks Item (which is 400
chars string) has random (and in the hanging case negative) number of chars
in it.

	Jakub

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Florian Weimer]

On 11/04/2015 02:13 PM, Jakub Jelinek wrote:
> On Mon, Jul 06, 2015 at 05:38:35PM +0100, Alan Lawrence wrote:
>> Trying to push these now (svn!), patch 2 is going first.
>>
>> I realize my second iteration of patch 1/2, dropped the testcases from the
>> first version. Okay to include those as per
>> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00215.html ?
> 
> FYI, it seems that (most likely) the PR65956 changes on gcc-5-branch
> broke libgnat ABI compatibility on arm - it seems that getsubs.adb
> from macrosub proglet (and others) are during make check compiled/linked
> with system gnatmake/gcc, but the program is run at runtime
> against the new libgnat-5.so.  If I run it manually against
> system libgnat, it works, otherwise it hangs, when Fill_Table from
> getsubs.adb calls Get_Line, and indeed it looks like the argument passing
> for Get_Line changed and on the callee side it thinks Item (which is 400
> chars string) has random (and in the hanging case negative) number of chars
> in it.

The patch looks at TYPE_MAIN_VARIANT without checking first if the type
has any qualifiers:

+  if (!AGGREGATE_TYPE_P (type))
+    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;

I'm not sure if this is valid, and what happens here if the type refers
to a fat pointer type generated by the Ada front end.

Florian

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

On 04/11/15 13:13, Jakub Jelinek wrote:
> On Mon, Jul 06, 2015 at 05:38:35PM +0100, Alan Lawrence wrote:
>> Trying to push these now (svn!), patch 2 is going first.
>>
>> I realize my second iteration of patch 1/2, dropped the testcases from the
>> first version. Okay to include those as per
>> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00215.html ?
>
> FYI, it seems that (most likely) the PR65956 changes on gcc-5-branch
> broke libgnat ABI compatibility on arm - it seems that getsubs.adb
> from macrosub proglet (and others) are during make check compiled/linked
> with system gnatmake/gcc, but the program is run at runtime
> against the new libgnat-5.so.  If I run it manually against
> system libgnat, it works, otherwise it hangs, when Fill_Table from
> getsubs.adb calls Get_Line, and indeed it looks like the argument passing
> for Get_Line changed and on the callee side it thinks Item (which is 400
> chars string) has random (and in the hanging case negative) number of chars
> in it.
>
> 	Jakub
>

Sorry Jakub, can you clarify please, how to reproduce this failure? I've just 
bootstrapped gcc-5-branch with ada and run the Ada testsuite, which has build me 
gcc/ada/rts/libgnat{.a,.so,-5.so}, and I see all tests passing. (Same with 
--disable-bootstrap FWIW.)

It seems plausible that Ada would be the language affected by the ABI change, 
obviously it would be somewhat ironic that we broke intercompatibility with 
gcc's own libgnat but not against libgnat prior to the change...

Thanks,
Alan

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Jakub Jelinek]

On Fri, Nov 06, 2015 at 04:48:02PM +0000, Alan Lawrence wrote:
> Sorry Jakub, can you clarify please, how to reproduce this failure? I've
> just bootstrapped gcc-5-branch with ada and run the Ada testsuite, which has
> build me gcc/ada/rts/libgnat{.a,.so,-5.so}, and I see all tests passing.
> (Same with --disable-bootstrap FWIW.)

I have installed a GCC 5.1.1 version including Ada on the system,
now bootstrap goes fine, but when doing make check the macrosub process just
hangs.  It happened to me only on Fedora 23/24 and not on Fedora 22.
I bet the issue why it sometimes can be reproduced and sometimes can't is
that due to the register passing changes Item'First and Item'Last are
uninitialized, and it the process only hangs if you are unlucky (the
difference Item'Last - Item'First is negative).

In any case, to manually reproduce, compile
gnatmake -g -gnatws macrosub.adb
with GCC 5.1.1 (before the ARM changes) and then try to run that process against
GCC 5.2.1 (after the ARM changes) libgnat-5.so, which is what make check
does (it uses host_gnatmake to compile the support stuff, so ideally the
processes built by host gcc/gnatmake should not be run with the
LD_LIBRARY_PATH=$ADA_INCLUDE_PATH:$BASE:$LD_LIBRARY_PATH
in the environment, and others should).
In macrosub in particular, the problem is in:
          WHILE NOT END_OF_FILE (INFILE1) LOOP
               GET_LINE (INFILE1, A_LINE, A_LENGTH);
in FILL_TABLE, where A_LINE'First is 0 and A_LINE'Last is 400 (if I remember
right), but if you step into GET_LINE compiled by GCC 5.2.1, Item'First
and Item'Last don't match that.

	Jakub

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

On 6 November 2015 at 16:59, Jakub Jelinek <jakub@redhat.com> wrote:
>
> In any case, to manually reproduce, compile
> gnatmake -g -gnatws macrosub.adb
> with GCC 5.1.1 (before the ARM changes) and then try to run that process against
> GCC 5.2.1 (after the ARM changes) libgnat-5.so, which is what make check
> does (it uses host_gnatmake to compile the support stuff, so ideally the
> processes built by host gcc/gnatmake should not be run with the
> LD_LIBRARY_PATH=$ADA_INCLUDE_PATH:$BASE:$LD_LIBRARY_PATH
> in the environment, and others should).
> In macrosub in particular, the problem is in:
>           WHILE NOT END_OF_FILE (INFILE1) LOOP
>                GET_LINE (INFILE1, A_LINE, A_LENGTH);
> in FILL_TABLE, where A_LINE'First is 0 and A_LINE'Last is 400 (if I remember
> right), but if you step into GET_LINE compiled by GCC 5.2.1, Item'First
> and Item'Last don't match that.

Ok, I see the mismatch now.

However, to get there, I had to use my 5.1 gnatmake -g -gnatws
macrosub.ads --rts=/path/to/5.2/arm-none-linux-gnueabihf/libada, as if
I ran 5.1 gnatmake without that flag, I did not manage to get the
wrong value passed/received with LD_LIBRARY_PATH set to any of
build-5.2/gcc/ada/rts, build-5.2/arm-none-linux-gnueabihf/libada,
build-5.2/arm-none-linux-gnueabihf/libada/adalib (any further
suggestions?). [Also I note 'LD_DEBUG=all ./macrosub' does not show
libgnat being loaded that way.]

With 5.1 gnatmake -g -gnatws macrosub.ads
--rts=/path/to/5.2/arm-none-linux-gnueabihf/libada :

$ gdb ./macrosub
GNU gdb (Ubuntu 7.7-0ubuntu3) 7.7
....[snip]....
Reading symbols from ./macrosub...done.
(gdb) break get_line
Breakpoint 1 at 0x1aeec: get_line. (4 locations)
(gdb) run
Starting program:
/home/alalaw01/build-5.1.0/gcc/testsuite/ada/acats/support/macrosub
BEGINNING MACRO SUBSTITUTIONS.

Breakpoint 1, ada.text_io.get_line (item=...) at a-tigeli.adb:41
41      procedure Get_Line
(gdb) print item'first
$1 = -443273216
(gdb) print item'last
$2 = -514850813
(gdb) n
146        FIO.Check_Read_Status (AP (File));
(gdb) n
152        if Item'First > Item'Last then
(gdb) print item'first
$3 = 1
(gdb) print item'last
$4 = 0
(gdb) up
#1  0x0001f34c in getsubs.fill_table () at getsubs.adb:122
122                    GET_LINE (INFILE1, A_LINE, A_LENGTH);
(gdb) print a_line'first
$5 = 1
(gdb) print a_line'last
$6 = 400

So yes, we have an ABI change; which is not entirely unexpected. So,
questions....

(1) Why does LD_LIBRARY_PATH affect your system, not mine (i.e. if
this is because my gnatmake is building with static linking, then
why). This is maybe the least interesting question so I'm leaving it
for now...
(2) If/when LD_LIBRARY_PATH does have an effect - as you say, things
compiled with host gnatmake, should be run against host libraries, not
against target libraries. Otherwise, potentially *any* gcc ABI change
can break the build process, right? So I think this is of interest
regardless of the ARM AAPCS change, but I will be slightly
presumptious and hope that the Adacore folk will pick this up...[CC
Eric]
(3) Has the ARM AAPCS had an effect that we didn't mean it to? I don't
see any evidence so far that this is _necessarily_ the case, but I
will look into this, bearing Florian's advice in mind (thanks!)...

--Alan

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Alan Lawrence]

On 26 November 2015 at 14:00, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On 6 November 2015 at 16:59, Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> In any case, to manually reproduce, compile
>> gnatmake -g -gnatws macrosub.adb
>> with GCC 5.1.1 (before the ARM changes) and then try to run that process against
>> GCC 5.2.1 (after the ARM changes) libgnat-5.so, which is what make check
>> does (it uses host_gnatmake to compile the support stuff, so ideally the
>> processes built by host gcc/gnatmake should not be run with the
>> LD_LIBRARY_PATH=$ADA_INCLUDE_PATH:$BASE:$LD_LIBRARY_PATH
>> in the environment, and others should).
>> In macrosub in particular, the problem is in:
>>           WHILE NOT END_OF_FILE (INFILE1) LOOP
>>                GET_LINE (INFILE1, A_LINE, A_LENGTH);
>> in FILL_TABLE, where A_LINE'First is 0 and A_LINE'Last is 400 (if I remember
>> right), but if you step into GET_LINE compiled by GCC 5.2.1, Item'First
>> and Item'Last don't match that.
>
> Ok, I see the mismatch now.

The type affected in Jakub's case here is an Ada String, which looks like this:

<record_type 0xf7569720 string___XUP sizes-gimplified asm_written
visited type_0 DI
    size <integer_cst 0xf7469210 type <integer_type 0xf7474060
bitsizetype> constant 64>
    unit size <integer_cst 0xf7469228 type <integer_type 0xf7474000
sizetype> constant 8>
    align 64 symtab -151604912 alias set -1 canonical type 0xf7569720
    fields <field_decl 0xf7569c60 P_ARRAY
        type <pointer_type 0xf756a2a0 type <array_type 0xf756a0c0 string___XUA>
            asm_written unsigned SI
            size <integer_cst 0xf74691b0 constant 32>
            unit size <integer_cst 0xf74691c8 constant 4>
            align 32 symtab -151604672 alias set -1 canonical type 0xf756a2a0>
        unsigned nonaddressable SI file <built-in> line 0 col 0 size
<integer_cst 0xf74691b0 32> unit size <integer_cst 0xf74691c8 4>
        align 32 offset_align 64
        offset <integer_cst 0xf74691e0 constant 0>
        bit offset <integer_cst 0xf7469240 constant 0> context
<record_type 0xf7569720 string___XUP>
        chain <field_decl 0xf7569cc0 P_BOUNDS type <pointer_type 0xf7569600>
            visited unsigned nonaddressable SI file <built-in> line 0
col 0 size <integer_cst 0xf74691b0 32> unit size <integer_cst
0xf74691c8 4>
            align 32 offset_align 64 offset <integer_cst 0xf74691e0 0>
bit offset <integer_cst 0xf74691b0 32> context <record_type 0xf7569720
string___XUP>>> context <translation_unit_decl 0xf77ea0a0 D.2757>
    unconstrained array <unconstrained_array_type 0xf7569c00 string
type <record_type 0xf7569720 string___XUP>
        BLK
        align 8 symtab 0 alias set -1 canonical type 0xf7569c00
context <translation_unit_decl 0xf77ea0a0 D.2757>
        pointer_to_this <record_type 0xf7569720 string___XUP>
reference_to_this <record_type 0xf7569720 string___XUP> chain
<type_decl 0xf756a660 string>>
    chain <type_decl 0xf7569d20 string___XUP>>

i.e. a 64-bit DImode struct, with alignment set to 64, containing
P_ARRAY a 32-bit pointer with alignment 32, and P_BOUNDS a 32-bit pointer
with alignment 32, pointing to a record (of size 64, alignment 32, containing
two 32-bit ints LB0 and UB0).

AFAICT, in the fill_table/get_line case, the first parameter to
get_line is a file, a simple pointer; then we have a string. So

*fill_table (compiled with 5.1, doubleword aligned) should pass the
string P_ARRAY in r2 and P_BOUNDS in r3.

0x1f334 <getsubs__fill_table+184>       movt   r3, #2
0x1f338 <getsubs__fill_table+188>       str    r3, [r11, #-504]        ; 0x
0x1f33c <getsubs__fill_table+192>       sub    r3, r11, #508   ; 0x1fc
0x1f340 <getsubs__fill_table+196>       ldrd   r2, [r3]
0x1f344 <getsubs__fill_table+200>       mov    r0, r1
0x1f348 <getsubs__fill_table+204>       bl     0x1aee4 <ada__text_io__get_line

looks plausible.

*get_line (compiled with 5.2, new AAPCS), should read P_ARRAY from r1,
and P_BOUNDS from r2. So the received P_BOUNDS is thus the passed
P_ARRAY, which in my example comes out as (1,0); the received P_ARRAY
is probably totally bogus.

And indeed the assembler for get_line seems to use r2 as a pointer to
a struct containing first & last.

So, I'm not familiar with Ada 'fat pointers' but if that is one -
well, it's a record, with an alignment that the 'new' AAPCS now
ignores, so yes the ABI has changed between gcc 5.1 and 5.2, rather
more significantly for Ada than for C.

Thoughts?


--Alan

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Eric Botcazou]

> So, I'm not familiar with Ada 'fat pointers' but if that is one -
> well, it's a record, with an alignment that the 'new' AAPCS now
> ignores, so yes the ABI has changed between gcc 5.1 and 5.2, rather
> more significantly for Ada than for C.

Yes, XUP suffixed types are fat pointers and they are maximally aligned so 
that they can be given non-BLK mode and, consequently, live in registers.

> Thoughts?

There is no official ABI for Ada so I guess that's not really a problem as 
long as it's documented on https://gcc.gnu.org/gcc-5/changes.html.

-- 
Eric Botcazou

Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute

[Florian Weimer]

On 11/27/2015 06:55 PM, Eric Botcazou wrote:

> There is no official ABI for Ada so I guess that's not really a problem as 
> long as it's documented on https://gcc.gnu.org/gcc-5/changes.html.

It's still surprising to make such a far-reaching change in a minor
release, I think.

Florian