| Message ID | 20170425100025.GU1809@tucnak |
|---|---|
| State | New |
| Headers | show |
On 25/04/17 11:00, Jakub Jelinek wrote: > Hi! > > As mentioned in the PR, r225465 aka PR65956 changed the ABI > on ARM to match updated AAPCS, but the change had a bug - for structures > it considered DECL_ALIGN of any TYPE_FIELDS, rather than just > actual data components (AAPCS says members, for C++ and Itanium C++ ABI > that is likely direct non-static data members and non-virtual base classes; > that means it also considered alignment of static data members (at least > this was consistent ABI difference), or DECL_ALIGN of TYPE_DECLs (which is > bigger problem, because that alignment is pretty randomish, it has different > value in types in templates depending on whether they have been instantiated > earlier or not)). > > The following patch fixes the ABI bug and adds -Wpsabi diagnostics (inform > rather than warning, so it doesn't break with -Werror and matches i386.c > -Wpsabi notes where there is no bug on the compiled code side). > > Earlier version of the patch has been bootstrapped/regtested on > armv7hl-linux-gnueabi, but there have been various changes since then. > Ok for trunk/7.1 if it passes testing? > > 2017-04-25 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> > Jakub Jelinek <jakub@redhat.com> > > PR target/77728 > * config/arm/arm.c: Include gimple.h. > (aapcs_layout_arg): Emit -Wpsabi note if arm_needs_doubleword_align > returns negative, increment ncrn only if it returned positive. > (arm_needs_doubleword_align): Return int instead of bool, > ignore DECL_ALIGN of non-FIELD_DECL TYPE_FIELDS chain > members, but if there is any such non-FIELD_DECL > > PARM_BOUNDARY aligned decl, return -1 instead of false. > (arm_function_arg): Emit -Wpsabi note if arm_needs_doubleword_align > returns negative, increment nregs only if it returned positive. > (arm_setup_incoming_varargs): Likewise. > (arm_function_arg_boundary): Emit -Wpsabi note if > arm_needs_doubleword_align returns negative, return > DOUBLEWORD_ALIGNMENT only if it returned positive. > testsuite/ > * g++.dg/abi/pr77728-1.C: New test. This is ok if it passes testing. R. > > --- gcc/config/arm/arm.c.jj 2017-04-25 09:20:49.740670794 +0200 > +++ gcc/config/arm/arm.c 2017-04-25 11:07:11.003121070 +0200 > @@ -64,6 +64,7 @@ > #include "rtl-iter.h" > #include "optabs-libfuncs.h" > #include "gimplify.h" > +#include "gimple.h" > > /* This file should be included last. */ > #include "target-def.h" > @@ -81,7 +82,7 @@ struct four_ints > > /* Forward function declarations. */ > static bool arm_const_not_ok_for_debug_p (rtx); > -static bool arm_needs_doubleword_align (machine_mode, const_tree); > +static int arm_needs_doubleword_align (machine_mode, const_tree); > static int arm_compute_static_chain_stack_bytes (void); > static arm_stack_offsets *arm_get_frame_offsets (void); > static void arm_add_gc_roots (void); > @@ -6349,8 +6350,20 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, > /* C3 - For double-word aligned arguments, round the NCRN up to the > next even number. */ > ncrn = pcum->aapcs_ncrn; > - if ((ncrn & 1) && arm_needs_doubleword_align (mode, type)) > - ncrn++; > + if (ncrn & 1) > + { > + int res = arm_needs_doubleword_align (mode, type); > + /* Only warn during RTL expansion of call stmts, otherwise we would > + warn e.g. during gimplification even on functions that will be > + always inlined, and we'd warn multiple times. Don't warn when > + called in expand_function_start either, as we warn instead in > + arm_function_arg_boundary in that case. */ > + if (res < 0 && warn_psabi && currently_expanding_gimple_stmt) > + inform (input_location, "parameter passing for argument of type " > + "%qT changed in GCC 7.1", type); > + else if (res > 0) > + ncrn++; > + } > > nregs = ARM_NUM_REGS2(mode, type); > > @@ -6455,12 +6468,16 @@ arm_init_cumulative_args (CUMULATIVE_ARG > } > } > > -/* Return true if mode/type need doubleword alignment. */ > -static bool > +/* Return 1 if double word alignment is required for argument passing. > + Return -1 if double word alignment used to be required for argument > + passing before PR77728 ABI fix, but is not required anymore. > + Return 0 if double word alignment is not required and wasn't requried > + before either. */ > +static int > arm_needs_doubleword_align (machine_mode mode, const_tree type) > { > if (!type) > - return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode); > + return GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY; > > /* Scalar and vector types: Use natural alignment, i.e. of base type. */ > if (!AGGREGATE_TYPE_P (type)) > @@ -6470,12 +6487,21 @@ arm_needs_doubleword_align (machine_mode > if (TREE_CODE (type) == ARRAY_TYPE) > return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY; > > + int ret = 0; > /* 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; > + { > + if (TREE_CODE (field) == FIELD_DECL) > + return 1; > + else > + /* Before PR77728 fix, we were incorrectly considering also > + other aggregate fields, like VAR_DECLs, TYPE_DECLs etc. > + Make sure we can warn about that with -Wpsabi. */ > + ret = -1; > + } > > - return false; > + return ret; > } > > > @@ -6532,10 +6558,15 @@ arm_function_arg (cumulative_args_t pcum > } > > /* Put doubleword aligned quantities in even register pairs. */ > - if (pcum->nregs & 1 > - && ARM_DOUBLEWORD_ALIGN > - && arm_needs_doubleword_align (mode, type)) > - pcum->nregs++; > + if ((pcum->nregs & 1) && ARM_DOUBLEWORD_ALIGN) > + { > + int res = arm_needs_doubleword_align (mode, type); > + if (res < 0 && warn_psabi) > + inform (input_location, "parameter passing for argument of type " > + "%qT changed in GCC 7.1", type); > + else if (res > 0) > + pcum->nregs++; > + } > > /* Only allow splitting an arg between regs and memory if all preceding > args were allocated to regs. For args passed by reference we only count > @@ -6554,9 +6585,15 @@ arm_function_arg (cumulative_args_t pcum > static unsigned int > arm_function_arg_boundary (machine_mode mode, const_tree type) > { > - return (ARM_DOUBLEWORD_ALIGN && arm_needs_doubleword_align (mode, type) > - ? DOUBLEWORD_ALIGNMENT > - : PARM_BOUNDARY); > + if (!ARM_DOUBLEWORD_ALIGN) > + return PARM_BOUNDARY; > + > + int res = arm_needs_doubleword_align (mode, type); > + if (res < 0 && warn_psabi) > + inform (input_location, "parameter passing for argument of type %qT " > + "changed in GCC 7.1", type); > + > + return res > 0 ? DOUBLEWORD_ALIGNMENT : PARM_BOUNDARY; > } > > static int > @@ -26516,8 +26553,15 @@ arm_setup_incoming_varargs (cumulative_a > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > nregs = pcum->aapcs_ncrn; > - if ((nregs & 1) && arm_needs_doubleword_align (mode, type)) > - nregs++; > + if (nregs & 1) > + { > + int res = arm_needs_doubleword_align (mode, type); > + if (res < 0 && warn_psabi) > + inform (input_location, "parameter passing for argument of " > + "type %qT changed in GCC 7.1", type); > + else if (res > 0) > + nregs++; > + } > } > else > nregs = pcum->nregs; > --- gcc/testsuite/g++.dg/abi/pr77728-1.C.jj 2017-04-25 09:30:39.262786365 +0200 > +++ gcc/testsuite/g++.dg/abi/pr77728-1.C 2017-04-25 10:14:59.134254239 +0200 > @@ -0,0 +1,171 @@ > +// { dg-do compile { target arm_eabi } } > +// { dg-options "-Wpsabi" } > + > +#include <stdarg.h> > + > +template <int N> > +struct A { double p; }; > + > +A<0> v; > + > +template <int N> > +struct B > +{ > + typedef A<N> T; > + int i, j; > +}; > + > +struct C : public B<0> {}; > +struct D {}; > +struct E : public D, C {}; > +struct F : public B<1> {}; > +struct G : public F { static double y; }; > +struct H : public G {}; > +struct I : public D { long long z; }; > +struct J : public D { static double z; int i, j; }; > + > +template <int N> > +struct K : public D { typedef A<N> T; int i, j; }; > + > +struct L { static double h; int i, j; }; > + > +int > +fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > +{ > + return a + b.i; > +} > + > +int > +fn2 (int a, B<1> b) > +{ > + return a + b.i; > +} > + > +int > +fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > +{ > + return a + b.i; > +} > + > +int > +fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...) > +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...) > +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...) > +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +void > +test () > +{ > + static B<0> b0; > + static B<1> b1; > + static L l; > + static C c; > + static E e; > + static H h; > + static I i; > + static J j; > + static K<0> k0; > + static K<2> k2; > + fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > + fn2 (1, b1); > + fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > + fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4); > + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > + fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4); > + fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4); > + fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4); > + fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4); > + fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4); > + fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4); > + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > + fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4); > + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > + fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4); > +} > > Jakub >
Hi all, On 25/04/17 11:00, Jakub Jelinek wrote: > Hi! > > As mentioned in the PR, r225465 aka PR65956 changed the ABI > on ARM to match updated AAPCS, but the change had a bug - for structures > it considered DECL_ALIGN of any TYPE_FIELDS, rather than just > actual data components (AAPCS says members, for C++ and Itanium C++ ABI > that is likely direct non-static data members and non-virtual base classes; > that means it also considered alignment of static data members (at least > this was consistent ABI difference), or DECL_ALIGN of TYPE_DECLs (which is > bigger problem, because that alignment is pretty randomish, it has different > value in types in templates depending on whether they have been instantiated > earlier or not)). > > The following patch fixes the ABI bug and adds -Wpsabi diagnostics (inform > rather than warning, so it doesn't break with -Werror and matches i386.c > -Wpsabi notes where there is no bug on the compiled code side). > > Earlier version of the patch has been bootstrapped/regtested on > armv7hl-linux-gnueabi, but there have been various changes since then. > Ok for trunk/7.1 if it passes testing? This patch passed bootstrap and testing on arm-none-linux-gnueabihf configured with --with-arch=armv7-a --with-fpu=neon --with-float=hard --with-mode=thumb. So the patch looks ok to me. Ramana, did you have testing running in another configuration that you wanted to report on? Thanks, Kyrill > 2017-04-25 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> > Jakub Jelinek <jakub@redhat.com> > > PR target/77728 > * config/arm/arm.c: Include gimple.h. > (aapcs_layout_arg): Emit -Wpsabi note if arm_needs_doubleword_align > returns negative, increment ncrn only if it returned positive. > (arm_needs_doubleword_align): Return int instead of bool, > ignore DECL_ALIGN of non-FIELD_DECL TYPE_FIELDS chain > members, but if there is any such non-FIELD_DECL > > PARM_BOUNDARY aligned decl, return -1 instead of false. > (arm_function_arg): Emit -Wpsabi note if arm_needs_doubleword_align > returns negative, increment nregs only if it returned positive. > (arm_setup_incoming_varargs): Likewise. > (arm_function_arg_boundary): Emit -Wpsabi note if > arm_needs_doubleword_align returns negative, return > DOUBLEWORD_ALIGNMENT only if it returned positive. > testsuite/ > * g++.dg/abi/pr77728-1.C: New test. > > --- gcc/config/arm/arm.c.jj 2017-04-25 09:20:49.740670794 +0200 > +++ gcc/config/arm/arm.c 2017-04-25 11:07:11.003121070 +0200 > @@ -64,6 +64,7 @@ > #include "rtl-iter.h" > #include "optabs-libfuncs.h" > #include "gimplify.h" > +#include "gimple.h" > > /* This file should be included last. */ > #include "target-def.h" > @@ -81,7 +82,7 @@ struct four_ints > > /* Forward function declarations. */ > static bool arm_const_not_ok_for_debug_p (rtx); > -static bool arm_needs_doubleword_align (machine_mode, const_tree); > +static int arm_needs_doubleword_align (machine_mode, const_tree); > static int arm_compute_static_chain_stack_bytes (void); > static arm_stack_offsets *arm_get_frame_offsets (void); > static void arm_add_gc_roots (void); > @@ -6349,8 +6350,20 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, > /* C3 - For double-word aligned arguments, round the NCRN up to the > next even number. */ > ncrn = pcum->aapcs_ncrn; > - if ((ncrn & 1) && arm_needs_doubleword_align (mode, type)) > - ncrn++; > + if (ncrn & 1) > + { > + int res = arm_needs_doubleword_align (mode, type); > + /* Only warn during RTL expansion of call stmts, otherwise we would > + warn e.g. during gimplification even on functions that will be > + always inlined, and we'd warn multiple times. Don't warn when > + called in expand_function_start either, as we warn instead in > + arm_function_arg_boundary in that case. */ > + if (res < 0 && warn_psabi && currently_expanding_gimple_stmt) > + inform (input_location, "parameter passing for argument of type " > + "%qT changed in GCC 7.1", type); > + else if (res > 0) > + ncrn++; > + } > > nregs = ARM_NUM_REGS2(mode, type); > > @@ -6455,12 +6468,16 @@ arm_init_cumulative_args (CUMULATIVE_ARG > } > } > > -/* Return true if mode/type need doubleword alignment. */ > -static bool > +/* Return 1 if double word alignment is required for argument passing. > + Return -1 if double word alignment used to be required for argument > + passing before PR77728 ABI fix, but is not required anymore. > + Return 0 if double word alignment is not required and wasn't requried > + before either. */ > +static int > arm_needs_doubleword_align (machine_mode mode, const_tree type) > { > if (!type) > - return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode); > + return GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY; > > /* Scalar and vector types: Use natural alignment, i.e. of base type. */ > if (!AGGREGATE_TYPE_P (type)) > @@ -6470,12 +6487,21 @@ arm_needs_doubleword_align (machine_mode > if (TREE_CODE (type) == ARRAY_TYPE) > return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY; > > + int ret = 0; > /* 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; > + { > + if (TREE_CODE (field) == FIELD_DECL) > + return 1; > + else > + /* Before PR77728 fix, we were incorrectly considering also > + other aggregate fields, like VAR_DECLs, TYPE_DECLs etc. > + Make sure we can warn about that with -Wpsabi. */ > + ret = -1; > + } > > - return false; > + return ret; > } > > > @@ -6532,10 +6558,15 @@ arm_function_arg (cumulative_args_t pcum > } > > /* Put doubleword aligned quantities in even register pairs. */ > - if (pcum->nregs & 1 > - && ARM_DOUBLEWORD_ALIGN > - && arm_needs_doubleword_align (mode, type)) > - pcum->nregs++; > + if ((pcum->nregs & 1) && ARM_DOUBLEWORD_ALIGN) > + { > + int res = arm_needs_doubleword_align (mode, type); > + if (res < 0 && warn_psabi) > + inform (input_location, "parameter passing for argument of type " > + "%qT changed in GCC 7.1", type); > + else if (res > 0) > + pcum->nregs++; > + } > > /* Only allow splitting an arg between regs and memory if all preceding > args were allocated to regs. For args passed by reference we only count > @@ -6554,9 +6585,15 @@ arm_function_arg (cumulative_args_t pcum > static unsigned int > arm_function_arg_boundary (machine_mode mode, const_tree type) > { > - return (ARM_DOUBLEWORD_ALIGN && arm_needs_doubleword_align (mode, type) > - ? DOUBLEWORD_ALIGNMENT > - : PARM_BOUNDARY); > + if (!ARM_DOUBLEWORD_ALIGN) > + return PARM_BOUNDARY; > + > + int res = arm_needs_doubleword_align (mode, type); > + if (res < 0 && warn_psabi) > + inform (input_location, "parameter passing for argument of type %qT " > + "changed in GCC 7.1", type); > + > + return res > 0 ? DOUBLEWORD_ALIGNMENT : PARM_BOUNDARY; > } > > static int > @@ -26516,8 +26553,15 @@ arm_setup_incoming_varargs (cumulative_a > if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) > { > nregs = pcum->aapcs_ncrn; > - if ((nregs & 1) && arm_needs_doubleword_align (mode, type)) > - nregs++; > + if (nregs & 1) > + { > + int res = arm_needs_doubleword_align (mode, type); > + if (res < 0 && warn_psabi) > + inform (input_location, "parameter passing for argument of " > + "type %qT changed in GCC 7.1", type); > + else if (res > 0) > + nregs++; > + } > } > else > nregs = pcum->nregs; > --- gcc/testsuite/g++.dg/abi/pr77728-1.C.jj 2017-04-25 09:30:39.262786365 +0200 > +++ gcc/testsuite/g++.dg/abi/pr77728-1.C 2017-04-25 10:14:59.134254239 +0200 > @@ -0,0 +1,171 @@ > +// { dg-do compile { target arm_eabi } } > +// { dg-options "-Wpsabi" } > + > +#include <stdarg.h> > + > +template <int N> > +struct A { double p; }; > + > +A<0> v; > + > +template <int N> > +struct B > +{ > + typedef A<N> T; > + int i, j; > +}; > + > +struct C : public B<0> {}; > +struct D {}; > +struct E : public D, C {}; > +struct F : public B<1> {}; > +struct G : public F { static double y; }; > +struct H : public G {}; > +struct I : public D { long long z; }; > +struct J : public D { static double z; int i, j; }; > + > +template <int N> > +struct K : public D { typedef A<N> T; int i, j; }; > + > +struct L { static double h; int i, j; }; > + > +int > +fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > +{ > + return a + b.i; > +} > + > +int > +fn2 (int a, B<1> b) > +{ > + return a + b.i; > +} > + > +int > +fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > +{ > + return a + b.i; > +} > + > +int > +fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...) > +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...) > +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...) > +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +int > +fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...) > +{ > + va_list ap; > + va_start (ap, n); > + int x = va_arg (ap, int); > + va_end (ap); > + return x; > +} > + > +void > +test () > +{ > + static B<0> b0; > + static B<1> b1; > + static L l; > + static C c; > + static E e; > + static H h; > + static I i; > + static J j; > + static K<0> k0; > + static K<2> k2; > + fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > + fn2 (1, b1); > + fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } > + fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4); > + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > + fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4); > + fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4); > + fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4); > + fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4); > + fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4); > + fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4); > + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > + fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4); > + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } > + fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4); > +} > > Jakub
On 25/04/17 14:57, Kyrill Tkachov wrote: > Hi all, > > On 25/04/17 11:00, Jakub Jelinek wrote: >> Hi! >> >> As mentioned in the PR, r225465 aka PR65956 changed the ABI >> on ARM to match updated AAPCS, but the change had a bug - for structures >> it considered DECL_ALIGN of any TYPE_FIELDS, rather than just >> actual data components (AAPCS says members, for C++ and Itanium C++ ABI >> that is likely direct non-static data members and non-virtual base classes; >> that means it also considered alignment of static data members (at least >> this was consistent ABI difference), or DECL_ALIGN of TYPE_DECLs (which is >> bigger problem, because that alignment is pretty randomish, it has different >> value in types in templates depending on whether they have been instantiated >> earlier or not)). >> >> The following patch fixes the ABI bug and adds -Wpsabi diagnostics (inform >> rather than warning, so it doesn't break with -Werror and matches i386.c >> -Wpsabi notes where there is no bug on the compiled code side). >> >> Earlier version of the patch has been bootstrapped/regtested on >> armv7hl-linux-gnueabi, but there have been various changes since then. >> Ok for trunk/7.1 if it passes testing? > > This patch passed bootstrap and testing on arm-none-linux-gnueabihf > configured with --with-arch=armv7-a --with-fpu=neon --with-float=hard --with-mode=thumb. > And the same with --with-mode=arm also passed bootstrap and is halfway through the testsuite I don't expect it to show any problems at this point (a previous full bootstrap and test with an earlier but functionally identical version of Ramana's patch didn't show any problems in this configuration). Thanks, Kyrill > So the patch looks ok to me. > Ramana, did you have testing running in another configuration that you wanted to report on? > > Thanks, > Kyrill > >> 2017-04-25 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> >> Jakub Jelinek <jakub@redhat.com> >> >> PR target/77728 >> * config/arm/arm.c: Include gimple.h. >> (aapcs_layout_arg): Emit -Wpsabi note if arm_needs_doubleword_align >> returns negative, increment ncrn only if it returned positive. >> (arm_needs_doubleword_align): Return int instead of bool, >> ignore DECL_ALIGN of non-FIELD_DECL TYPE_FIELDS chain >> members, but if there is any such non-FIELD_DECL >> > PARM_BOUNDARY aligned decl, return -1 instead of false. >> (arm_function_arg): Emit -Wpsabi note if arm_needs_doubleword_align >> returns negative, increment nregs only if it returned positive. >> (arm_setup_incoming_varargs): Likewise. >> (arm_function_arg_boundary): Emit -Wpsabi note if >> arm_needs_doubleword_align returns negative, return >> DOUBLEWORD_ALIGNMENT only if it returned positive. >> testsuite/ >> * g++.dg/abi/pr77728-1.C: New test. >> >> --- gcc/config/arm/arm.c.jj 2017-04-25 09:20:49.740670794 +0200 >> +++ gcc/config/arm/arm.c 2017-04-25 11:07:11.003121070 +0200 >> @@ -64,6 +64,7 @@ >> #include "rtl-iter.h" >> #include "optabs-libfuncs.h" >> #include "gimplify.h" >> +#include "gimple.h" >> /* This file should be included last. */ >> #include "target-def.h" >> @@ -81,7 +82,7 @@ struct four_ints >> /* Forward function declarations. */ >> static bool arm_const_not_ok_for_debug_p (rtx); >> -static bool arm_needs_doubleword_align (machine_mode, const_tree); >> +static int arm_needs_doubleword_align (machine_mode, const_tree); >> static int arm_compute_static_chain_stack_bytes (void); >> static arm_stack_offsets *arm_get_frame_offsets (void); >> static void arm_add_gc_roots (void); >> @@ -6349,8 +6350,20 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, >> /* C3 - For double-word aligned arguments, round the NCRN up to the >> next even number. */ >> ncrn = pcum->aapcs_ncrn; >> - if ((ncrn & 1) && arm_needs_doubleword_align (mode, type)) >> - ncrn++; >> + if (ncrn & 1) >> + { >> + int res = arm_needs_doubleword_align (mode, type); >> + /* Only warn during RTL expansion of call stmts, otherwise we would >> + warn e.g. during gimplification even on functions that will be >> + always inlined, and we'd warn multiple times. Don't warn when >> + called in expand_function_start either, as we warn instead in >> + arm_function_arg_boundary in that case. */ >> + if (res < 0 && warn_psabi && currently_expanding_gimple_stmt) >> + inform (input_location, "parameter passing for argument of type " >> + "%qT changed in GCC 7.1", type); >> + else if (res > 0) >> + ncrn++; >> + } >> nregs = ARM_NUM_REGS2(mode, type); >> @@ -6455,12 +6468,16 @@ arm_init_cumulative_args (CUMULATIVE_ARG >> } >> } >> -/* Return true if mode/type need doubleword alignment. */ >> -static bool >> +/* Return 1 if double word alignment is required for argument passing. >> + Return -1 if double word alignment used to be required for argument >> + passing before PR77728 ABI fix, but is not required anymore. >> + Return 0 if double word alignment is not required and wasn't requried >> + before either. */ >> +static int >> arm_needs_doubleword_align (machine_mode mode, const_tree type) >> { >> if (!type) >> - return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode); >> + return GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY; >> /* Scalar and vector types: Use natural alignment, i.e. of base type. */ >> if (!AGGREGATE_TYPE_P (type)) >> @@ -6470,12 +6487,21 @@ arm_needs_doubleword_align (machine_mode >> if (TREE_CODE (type) == ARRAY_TYPE) >> return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY; >> + int ret = 0; >> /* 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; >> + { >> + if (TREE_CODE (field) == FIELD_DECL) >> + return 1; >> + else >> + /* Before PR77728 fix, we were incorrectly considering also >> + other aggregate fields, like VAR_DECLs, TYPE_DECLs etc. >> + Make sure we can warn about that with -Wpsabi. */ >> + ret = -1; >> + } >> - return false; >> + return ret; >> } >> @@ -6532,10 +6558,15 @@ arm_function_arg (cumulative_args_t pcum >> } >> /* Put doubleword aligned quantities in even register pairs. */ >> - if (pcum->nregs & 1 >> - && ARM_DOUBLEWORD_ALIGN >> - && arm_needs_doubleword_align (mode, type)) >> - pcum->nregs++; >> + if ((pcum->nregs & 1) && ARM_DOUBLEWORD_ALIGN) >> + { >> + int res = arm_needs_doubleword_align (mode, type); >> + if (res < 0 && warn_psabi) >> + inform (input_location, "parameter passing for argument of type " >> + "%qT changed in GCC 7.1", type); >> + else if (res > 0) >> + pcum->nregs++; >> + } >> /* Only allow splitting an arg between regs and memory if all preceding >> args were allocated to regs. For args passed by reference we only count >> @@ -6554,9 +6585,15 @@ arm_function_arg (cumulative_args_t pcum >> static unsigned int >> arm_function_arg_boundary (machine_mode mode, const_tree type) >> { >> - return (ARM_DOUBLEWORD_ALIGN && arm_needs_doubleword_align (mode, type) >> - ? DOUBLEWORD_ALIGNMENT >> - : PARM_BOUNDARY); >> + if (!ARM_DOUBLEWORD_ALIGN) >> + return PARM_BOUNDARY; >> + >> + int res = arm_needs_doubleword_align (mode, type); >> + if (res < 0 && warn_psabi) >> + inform (input_location, "parameter passing for argument of type %qT " >> + "changed in GCC 7.1", type); >> + >> + return res > 0 ? DOUBLEWORD_ALIGNMENT : PARM_BOUNDARY; >> } >> static int >> @@ -26516,8 +26553,15 @@ arm_setup_incoming_varargs (cumulative_a >> if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) >> { >> nregs = pcum->aapcs_ncrn; >> - if ((nregs & 1) && arm_needs_doubleword_align (mode, type)) >> - nregs++; >> + if (nregs & 1) >> + { >> + int res = arm_needs_doubleword_align (mode, type); >> + if (res < 0 && warn_psabi) >> + inform (input_location, "parameter passing for argument of " >> + "type %qT changed in GCC 7.1", type); >> + else if (res > 0) >> + nregs++; >> + } >> } >> else >> nregs = pcum->nregs; >> --- gcc/testsuite/g++.dg/abi/pr77728-1.C.jj 2017-04-25 09:30:39.262786365 +0200 >> +++ gcc/testsuite/g++.dg/abi/pr77728-1.C 2017-04-25 10:14:59.134254239 +0200 >> @@ -0,0 +1,171 @@ >> +// { dg-do compile { target arm_eabi } } >> +// { dg-options "-Wpsabi" } >> + >> +#include <stdarg.h> >> + >> +template <int N> >> +struct A { double p; }; >> + >> +A<0> v; >> + >> +template <int N> >> +struct B >> +{ >> + typedef A<N> T; >> + int i, j; >> +}; >> + >> +struct C : public B<0> {}; >> +struct D {}; >> +struct E : public D, C {}; >> +struct F : public B<1> {}; >> +struct G : public F { static double y; }; >> +struct H : public G {}; >> +struct I : public D { long long z; }; >> +struct J : public D { static double z; int i, j; }; >> + >> +template <int N> >> +struct K : public D { typedef A<N> T; int i, j; }; >> + >> +struct L { static double h; int i, j; }; >> + >> +int >> +fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } >> +{ >> + return a + b.i; >> +} >> + >> +int >> +fn2 (int a, B<1> b) >> +{ >> + return a + b.i; >> +} >> + >> +int >> +fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } >> +{ >> + return a + b.i; >> +} >> + >> +int >> +fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...) >> +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...) >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...) >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...) >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...) >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...) >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...) >> +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...) >> +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +int >> +fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...) >> +{ >> + va_list ap; >> + va_start (ap, n); >> + int x = va_arg (ap, int); >> + va_end (ap); >> + return x; >> +} >> + >> +void >> +test () >> +{ >> + static B<0> b0; >> + static B<1> b1; >> + static L l; >> + static C c; >> + static E e; >> + static H h; >> + static I i; >> + static J j; >> + static K<0> k0; >> + static K<2> k2; >> + fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } >> + fn2 (1, b1); >> + fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } >> + fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4); >> + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } >> + fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4); >> + fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4); >> + fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4); >> + fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4); >> + fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4); >> + fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4); >> + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } >> + fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4); >> + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } >> + fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4); >> +} >> >> Jakub >
--- gcc/config/arm/arm.c.jj 2017-04-25 09:20:49.740670794 +0200 +++ gcc/config/arm/arm.c 2017-04-25 11:07:11.003121070 +0200 @@ -64,6 +64,7 @@ #include "rtl-iter.h" #include "optabs-libfuncs.h" #include "gimplify.h" +#include "gimple.h" /* This file should be included last. */ #include "target-def.h" @@ -81,7 +82,7 @@ struct four_ints /* Forward function declarations. */ static bool arm_const_not_ok_for_debug_p (rtx); -static bool arm_needs_doubleword_align (machine_mode, const_tree); +static int arm_needs_doubleword_align (machine_mode, const_tree); static int arm_compute_static_chain_stack_bytes (void); static arm_stack_offsets *arm_get_frame_offsets (void); static void arm_add_gc_roots (void); @@ -6349,8 +6350,20 @@ aapcs_layout_arg (CUMULATIVE_ARGS *pcum, /* C3 - For double-word aligned arguments, round the NCRN up to the next even number. */ ncrn = pcum->aapcs_ncrn; - if ((ncrn & 1) && arm_needs_doubleword_align (mode, type)) - ncrn++; + if (ncrn & 1) + { + int res = arm_needs_doubleword_align (mode, type); + /* Only warn during RTL expansion of call stmts, otherwise we would + warn e.g. during gimplification even on functions that will be + always inlined, and we'd warn multiple times. Don't warn when + called in expand_function_start either, as we warn instead in + arm_function_arg_boundary in that case. */ + if (res < 0 && warn_psabi && currently_expanding_gimple_stmt) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 7.1", type); + else if (res > 0) + ncrn++; + } nregs = ARM_NUM_REGS2(mode, type); @@ -6455,12 +6468,16 @@ arm_init_cumulative_args (CUMULATIVE_ARG } } -/* Return true if mode/type need doubleword alignment. */ -static bool +/* Return 1 if double word alignment is required for argument passing. + Return -1 if double word alignment used to be required for argument + passing before PR77728 ABI fix, but is not required anymore. + Return 0 if double word alignment is not required and wasn't requried + before either. */ +static int arm_needs_doubleword_align (machine_mode mode, const_tree type) { if (!type) - return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode); + return GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY; /* Scalar and vector types: Use natural alignment, i.e. of base type. */ if (!AGGREGATE_TYPE_P (type)) @@ -6470,12 +6487,21 @@ arm_needs_doubleword_align (machine_mode if (TREE_CODE (type) == ARRAY_TYPE) return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY; + int ret = 0; /* 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; + { + if (TREE_CODE (field) == FIELD_DECL) + return 1; + else + /* Before PR77728 fix, we were incorrectly considering also + other aggregate fields, like VAR_DECLs, TYPE_DECLs etc. + Make sure we can warn about that with -Wpsabi. */ + ret = -1; + } - return false; + return ret; } @@ -6532,10 +6558,15 @@ arm_function_arg (cumulative_args_t pcum } /* Put doubleword aligned quantities in even register pairs. */ - if (pcum->nregs & 1 - && ARM_DOUBLEWORD_ALIGN - && arm_needs_doubleword_align (mode, type)) - pcum->nregs++; + if ((pcum->nregs & 1) && ARM_DOUBLEWORD_ALIGN) + { + int res = arm_needs_doubleword_align (mode, type); + if (res < 0 && warn_psabi) + inform (input_location, "parameter passing for argument of type " + "%qT changed in GCC 7.1", type); + else if (res > 0) + pcum->nregs++; + } /* Only allow splitting an arg between regs and memory if all preceding args were allocated to regs. For args passed by reference we only count @@ -6554,9 +6585,15 @@ arm_function_arg (cumulative_args_t pcum static unsigned int arm_function_arg_boundary (machine_mode mode, const_tree type) { - return (ARM_DOUBLEWORD_ALIGN && arm_needs_doubleword_align (mode, type) - ? DOUBLEWORD_ALIGNMENT - : PARM_BOUNDARY); + if (!ARM_DOUBLEWORD_ALIGN) + return PARM_BOUNDARY; + + int res = arm_needs_doubleword_align (mode, type); + if (res < 0 && warn_psabi) + inform (input_location, "parameter passing for argument of type %qT " + "changed in GCC 7.1", type); + + return res > 0 ? DOUBLEWORD_ALIGNMENT : PARM_BOUNDARY; } static int @@ -26516,8 +26553,15 @@ arm_setup_incoming_varargs (cumulative_a if (pcum->pcs_variant <= ARM_PCS_AAPCS_LOCAL) { nregs = pcum->aapcs_ncrn; - if ((nregs & 1) && arm_needs_doubleword_align (mode, type)) - nregs++; + if (nregs & 1) + { + int res = arm_needs_doubleword_align (mode, type); + if (res < 0 && warn_psabi) + inform (input_location, "parameter passing for argument of " + "type %qT changed in GCC 7.1", type); + else if (res > 0) + nregs++; + } } else nregs = pcum->nregs; --- gcc/testsuite/g++.dg/abi/pr77728-1.C.jj 2017-04-25 09:30:39.262786365 +0200 +++ gcc/testsuite/g++.dg/abi/pr77728-1.C 2017-04-25 10:14:59.134254239 +0200 @@ -0,0 +1,171 @@ +// { dg-do compile { target arm_eabi } } +// { dg-options "-Wpsabi" } + +#include <stdarg.h> + +template <int N> +struct A { double p; }; + +A<0> v; + +template <int N> +struct B +{ + typedef A<N> T; + int i, j; +}; + +struct C : public B<0> {}; +struct D {}; +struct E : public D, C {}; +struct F : public B<1> {}; +struct G : public F { static double y; }; +struct H : public G {}; +struct I : public D { long long z; }; +struct J : public D { static double z; int i, j; }; + +template <int N> +struct K : public D { typedef A<N> T; int i, j; }; + +struct L { static double h; int i, j; }; + +int +fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } +{ + return a + b.i; +} + +int +fn2 (int a, B<1> b) +{ + return a + b.i; +} + +int +fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } +{ + return a + b.i; +} + +int +fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...) +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...) +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...) +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +int +fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...) +{ + va_list ap; + va_start (ap, n); + int x = va_arg (ap, int); + va_end (ap); + return x; +} + +void +test () +{ + static B<0> b0; + static B<1> b1; + static L l; + static C c; + static E e; + static H h; + static I i; + static J j; + static K<0> k0; + static K<2> k2; + fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } + fn2 (1, b1); + fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" } + fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4); + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } + fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4); + fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4); + fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4); + fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4); + fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4); + fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4); + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } + fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4); + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 } + fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4); +}
Hi! As mentioned in the PR, r225465 aka PR65956 changed the ABI on ARM to match updated AAPCS, but the change had a bug - for structures it considered DECL_ALIGN of any TYPE_FIELDS, rather than just actual data components (AAPCS says members, for C++ and Itanium C++ ABI that is likely direct non-static data members and non-virtual base classes; that means it also considered alignment of static data members (at least this was consistent ABI difference), or DECL_ALIGN of TYPE_DECLs (which is bigger problem, because that alignment is pretty randomish, it has different value in types in templates depending on whether they have been instantiated earlier or not)). The following patch fixes the ABI bug and adds -Wpsabi diagnostics (inform rather than warning, so it doesn't break with -Werror and matches i386.c -Wpsabi notes where there is no bug on the compiled code side). Earlier version of the patch has been bootstrapped/regtested on armv7hl-linux-gnueabi, but there have been various changes since then. Ok for trunk/7.1 if it passes testing? 2017-04-25 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> Jakub Jelinek <jakub@redhat.com> PR target/77728 * config/arm/arm.c: Include gimple.h. (aapcs_layout_arg): Emit -Wpsabi note if arm_needs_doubleword_align returns negative, increment ncrn only if it returned positive. (arm_needs_doubleword_align): Return int instead of bool, ignore DECL_ALIGN of non-FIELD_DECL TYPE_FIELDS chain members, but if there is any such non-FIELD_DECL > PARM_BOUNDARY aligned decl, return -1 instead of false. (arm_function_arg): Emit -Wpsabi note if arm_needs_doubleword_align returns negative, increment nregs only if it returned positive. (arm_setup_incoming_varargs): Likewise. (arm_function_arg_boundary): Emit -Wpsabi note if arm_needs_doubleword_align returns negative, return DOUBLEWORD_ALIGNMENT only if it returned positive. testsuite/ * g++.dg/abi/pr77728-1.C: New test. Jakub