Message ID | 20230602191153.78156-1-iain@sandoe.co.uk |
---|---|
State | New |
Headers | show |
Series | [pushed] Darwin, PPC: Fix struct layout with pragma pack [PR110044]. | expand |
> Am 02.06.2023 um 21:12 schrieb Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org>: > > @David: I am not sure what sets the ABI on AIX (for Darwin, it is effectively > "whatever the system compiler [Apple gcc-4] does") but from an inspection of > the code, it seems that (if the platform should honour #pragma pack) a similar > effect could be present there too. > > Tested on powerpc-apple-darwin9, powerpc64-linux-gnu and on i686 and x86_64 > Darwin. Checked that the testcases also pass for Apple gcc-4.2.1. > pushed to trunk, thanks > Iain > > --- 8< --- > > This bug was essentially that darwin_rs6000_special_round_type_align() > was ignoring externally-imposed capping of field alignment. > > Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > > PR target/110044 > > gcc/ChangeLog: > > * config/rs6000/rs6000.cc (darwin_rs6000_special_round_type_align): > Make sure that we do not have a cap on field alignment before altering > the struct layout based on the type alignment of the first entry. > > gcc/testsuite/ChangeLog: > > * gcc.target/powerpc/darwin-abi-13-0.c: New test. > * gcc.target/powerpc/darwin-abi-13-1.c: New test. > * gcc.target/powerpc/darwin-abi-13-2.c: New test. > * gcc.target/powerpc/darwin-structs-0.h: New test. > --- > gcc/config/rs6000/rs6000.cc | 3 +- > .../gcc.target/powerpc/darwin-abi-13-0.c | 23 +++++++++++++++ > .../gcc.target/powerpc/darwin-abi-13-1.c | 27 +++++++++++++++++ > .../gcc.target/powerpc/darwin-abi-13-2.c | 27 +++++++++++++++++ > .../gcc.target/powerpc/darwin-structs-0.h | 29 +++++++++++++++++++ > 5 files changed, 108 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c > create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c > create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c > create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index 5b3b8b52e7e..42f49e4a56b 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -8209,7 +8209,8 @@ darwin_rs6000_special_round_type_align (tree type, unsigned int computed, > type = TREE_TYPE (type); > } while (AGGREGATE_TYPE_P (type)); > > - if (! AGGREGATE_TYPE_P (type) && type != error_mark_node) > + if (type != error_mark_node && ! AGGREGATE_TYPE_P (type) > + && ! TYPE_PACKED (type) && maximum_field_alignment == 0) Just noticed while browsing mail. ‚Maximum_field_alignment‘ sounds like Something that should be factored in when Computing align but as written there’s no adjustment done instead? Is there a way to get that to more than BITS_PER_UNIT? > align = MAX (align, TYPE_ALIGN (type)); > > return align; > diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c > new file mode 100644 > index 00000000000..d8d3c63a083 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c > @@ -0,0 +1,23 @@ > +/* { dg-do compile { target powerpc*-*-darwin* } } */ > +/* { dg-require-effective-target ilp32 } */ > +/* { dg-options "-Wno-long-long" } */ > + > +#include "darwin-structs-0.h" > + > +int tcd[sizeof(cd) != 12 ? -1 : 1]; > +int acd[__alignof__(cd) != 4 ? -1 : 1]; > + > +int sdc[sizeof(dc) != 16 ? -1 : 1]; > +int adc[__alignof__(dc) != 8 ? -1 : 1]; > + > +int scL[sizeof(cL) != 12 ? -1 : 1]; > +int acL[__alignof__(cL) != 4 ? -1 : 1]; > + > +int sLc[sizeof(Lc) != 16 ? -1 : 1]; > +int aLc[__alignof__(Lc) != 8 ? -1 : 1]; > + > +int scD[sizeof(cD) != 32 ? -1 : 1]; > +int acD[__alignof__(cD) != 16 ? -1 : 1]; > + > +int sDc[sizeof(Dc) != 32 ? -1 : 1]; > +int aDc[__alignof__(Dc) != 16 ? -1 : 1]; > diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c > new file mode 100644 > index 00000000000..4d888d383fa > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c > @@ -0,0 +1,27 @@ > +/* { dg-do compile { target powerpc*-*-darwin* } } */ > +/* { dg-require-effective-target ilp32 } */ > +/* { dg-options "-Wno-long-long" } */ > + > +#pragma pack(push, 1) > + > +#include "darwin-structs-0.h" > + > +int tcd[sizeof(cd) != 9 ? -1 : 1]; > +int acd[__alignof__(cd) != 1 ? -1 : 1]; > + > +int sdc[sizeof(dc) != 9 ? -1 : 1]; > +int adc[__alignof__(dc) != 1 ? -1 : 1]; > + > +int scL[sizeof(cL) != 9 ? -1 : 1]; > +int acL[__alignof__(cL) != 1 ? -1 : 1]; > + > +int sLc[sizeof(Lc) != 9 ? -1 : 1]; > +int aLc[__alignof__(Lc) != 1 ? -1 : 1]; > + > +int scD[sizeof(cD) != 17 ? -1 : 1]; > +int acD[__alignof__(cD) != 1 ? -1 : 1]; > + > +int sDc[sizeof(Dc) != 17 ? -1 : 1]; > +int aDc[__alignof__(Dc) != 1 ? -1 : 1]; > + > +#pragma pack(pop) > diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c > new file mode 100644 > index 00000000000..3bd52c0a8f8 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c > @@ -0,0 +1,27 @@ > +/* { dg-do compile { target powerpc*-*-darwin* } } */ > +/* { dg-require-effective-target ilp32 } */ > +/* { dg-options "-Wno-long-long" } */ > + > +#pragma pack(push, 2) > + > +#include "darwin-structs-0.h" > + > +int tcd[sizeof(cd) != 10 ? -1 : 1]; > +int acd[__alignof__(cd) != 2 ? -1 : 1]; > + > +int sdc[sizeof(dc) != 10 ? -1 : 1]; > +int adc[__alignof__(dc) != 2 ? -1 : 1]; > + > +int scL[sizeof(cL) != 10 ? -1 : 1]; > +int acL[__alignof__(cL) != 2 ? -1 : 1]; > + > +int sLc[sizeof(Lc) != 10 ? -1 : 1]; > +int aLc[__alignof__(Lc) != 2 ? -1 : 1]; > + > +int scD[sizeof(cD) != 18 ? -1 : 1]; > +int acD[__alignof__(cD) != 2 ? -1 : 1]; > + > +int sDc[sizeof(Dc) != 18 ? -1 : 1]; > +int aDc[__alignof__(Dc) != 2 ? -1 : 1]; > + > +#pragma pack(pop) > diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h b/gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h > new file mode 100644 > index 00000000000..1db44f7a808 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h > @@ -0,0 +1,29 @@ > +typedef struct _cd { > + char c; > + double d; > +} cd; > + > +typedef struct _dc { > + double d; > + char c; > +} dc; > + > +typedef struct _cL { > + char c; > + long long L; > +} cL; > + > +typedef struct _Lc { > + long long L; > + char c; > +} Lc; > + > +typedef struct _cD { > + char c; > + long double D; > +} cD; > + > +typedef struct _Dc { > + long double D; > + char c; > +} Dc; > -- > 2.39.2 (Apple Git-143) >
Hi Richard, > On 3 Jun 2023, at 12:20, Richard Biener <richard.guenther@gmail.com> wrote: > >> Am 02.06.2023 um 21:12 schrieb Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org>: >> >> --- 8< --- >> >> This bug was essentially that darwin_rs6000_special_round_type_align() >> was ignoring externally-imposed capping of field alignment. >> >> >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >> index 5b3b8b52e7e..42f49e4a56b 100644 >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -8209,7 +8209,8 @@ darwin_rs6000_special_round_type_align (tree type, unsigned int computed, >> type = TREE_TYPE (type); >> } while (AGGREGATE_TYPE_P (type)); >> >> - if (! AGGREGATE_TYPE_P (type) && type != error_mark_node) >> + if (type != error_mark_node && ! AGGREGATE_TYPE_P (type) >> + && ! TYPE_PACKED (type) && maximum_field_alignment == 0) > > Just noticed while browsing mail. ‚Maximum_field_alignment‘ sounds like > Something that should be factored in when > Computing align but as written there’s no adjustment done instead? Is there a way to get that to more than BITS_PER_UNIT? I believe it is already correctly factored in (the values of ‘computed’ and ’specified’ supplied to the darwin_rs6000_special_round_type_align() take it into account). The point of this function is to override the supplied values under specific conditions (that the first element in the aggregate is a double or long long). However, [at least in the de facto Darwin PPC32 ABI] we should not do so if there is a packed pragma in effect (that takes priority) and omitting that check is the bug being fixed. It is a bit unfortunate to be looking at a global from deep in the machinery (although I did a quick grep and it seems that this would not be easily fixable - several targets and other places do inspect maximum_field_alignment). I suppose we could add a parm indicating the packed status and/or value. Part of the motivation for a self-contained and Darwin-local solution is to allow backport to 10.x before it closes (since that’s the last GCC branch that can be built with native tools on the earlier boxes). hopefully, I understood your point? cheers Iain
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 5b3b8b52e7e..42f49e4a56b 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -8209,7 +8209,8 @@ darwin_rs6000_special_round_type_align (tree type, unsigned int computed, type = TREE_TYPE (type); } while (AGGREGATE_TYPE_P (type)); - if (! AGGREGATE_TYPE_P (type) && type != error_mark_node) + if (type != error_mark_node && ! AGGREGATE_TYPE_P (type) + && ! TYPE_PACKED (type) && maximum_field_alignment == 0) align = MAX (align, TYPE_ALIGN (type)); return align; diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c new file mode 100644 index 00000000000..d8d3c63a083 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c @@ -0,0 +1,23 @@ +/* { dg-do compile { target powerpc*-*-darwin* } } */ +/* { dg-require-effective-target ilp32 } */ +/* { dg-options "-Wno-long-long" } */ + +#include "darwin-structs-0.h" + +int tcd[sizeof(cd) != 12 ? -1 : 1]; +int acd[__alignof__(cd) != 4 ? -1 : 1]; + +int sdc[sizeof(dc) != 16 ? -1 : 1]; +int adc[__alignof__(dc) != 8 ? -1 : 1]; + +int scL[sizeof(cL) != 12 ? -1 : 1]; +int acL[__alignof__(cL) != 4 ? -1 : 1]; + +int sLc[sizeof(Lc) != 16 ? -1 : 1]; +int aLc[__alignof__(Lc) != 8 ? -1 : 1]; + +int scD[sizeof(cD) != 32 ? -1 : 1]; +int acD[__alignof__(cD) != 16 ? -1 : 1]; + +int sDc[sizeof(Dc) != 32 ? -1 : 1]; +int aDc[__alignof__(Dc) != 16 ? -1 : 1]; diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c new file mode 100644 index 00000000000..4d888d383fa --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c @@ -0,0 +1,27 @@ +/* { dg-do compile { target powerpc*-*-darwin* } } */ +/* { dg-require-effective-target ilp32 } */ +/* { dg-options "-Wno-long-long" } */ + +#pragma pack(push, 1) + +#include "darwin-structs-0.h" + +int tcd[sizeof(cd) != 9 ? -1 : 1]; +int acd[__alignof__(cd) != 1 ? -1 : 1]; + +int sdc[sizeof(dc) != 9 ? -1 : 1]; +int adc[__alignof__(dc) != 1 ? -1 : 1]; + +int scL[sizeof(cL) != 9 ? -1 : 1]; +int acL[__alignof__(cL) != 1 ? -1 : 1]; + +int sLc[sizeof(Lc) != 9 ? -1 : 1]; +int aLc[__alignof__(Lc) != 1 ? -1 : 1]; + +int scD[sizeof(cD) != 17 ? -1 : 1]; +int acD[__alignof__(cD) != 1 ? -1 : 1]; + +int sDc[sizeof(Dc) != 17 ? -1 : 1]; +int aDc[__alignof__(Dc) != 1 ? -1 : 1]; + +#pragma pack(pop) diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c new file mode 100644 index 00000000000..3bd52c0a8f8 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c @@ -0,0 +1,27 @@ +/* { dg-do compile { target powerpc*-*-darwin* } } */ +/* { dg-require-effective-target ilp32 } */ +/* { dg-options "-Wno-long-long" } */ + +#pragma pack(push, 2) + +#include "darwin-structs-0.h" + +int tcd[sizeof(cd) != 10 ? -1 : 1]; +int acd[__alignof__(cd) != 2 ? -1 : 1]; + +int sdc[sizeof(dc) != 10 ? -1 : 1]; +int adc[__alignof__(dc) != 2 ? -1 : 1]; + +int scL[sizeof(cL) != 10 ? -1 : 1]; +int acL[__alignof__(cL) != 2 ? -1 : 1]; + +int sLc[sizeof(Lc) != 10 ? -1 : 1]; +int aLc[__alignof__(Lc) != 2 ? -1 : 1]; + +int scD[sizeof(cD) != 18 ? -1 : 1]; +int acD[__alignof__(cD) != 2 ? -1 : 1]; + +int sDc[sizeof(Dc) != 18 ? -1 : 1]; +int aDc[__alignof__(Dc) != 2 ? -1 : 1]; + +#pragma pack(pop) diff --git a/gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h b/gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h new file mode 100644 index 00000000000..1db44f7a808 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h @@ -0,0 +1,29 @@ +typedef struct _cd { + char c; + double d; +} cd; + +typedef struct _dc { + double d; + char c; +} dc; + +typedef struct _cL { + char c; + long long L; +} cL; + +typedef struct _Lc { + long long L; + char c; +} Lc; + +typedef struct _cD { + char c; + long double D; +} cD; + +typedef struct _Dc { + long double D; + char c; +} Dc;
@David: I am not sure what sets the ABI on AIX (for Darwin, it is effectively "whatever the system compiler [Apple gcc-4] does") but from an inspection of the code, it seems that (if the platform should honour #pragma pack) a similar effect could be present there too. Tested on powerpc-apple-darwin9, powerpc64-linux-gnu and on i686 and x86_64 Darwin. Checked that the testcases also pass for Apple gcc-4.2.1. pushed to trunk, thanks Iain --- 8< --- This bug was essentially that darwin_rs6000_special_round_type_align() was ignoring externally-imposed capping of field alignment. Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> PR target/110044 gcc/ChangeLog: * config/rs6000/rs6000.cc (darwin_rs6000_special_round_type_align): Make sure that we do not have a cap on field alignment before altering the struct layout based on the type alignment of the first entry. gcc/testsuite/ChangeLog: * gcc.target/powerpc/darwin-abi-13-0.c: New test. * gcc.target/powerpc/darwin-abi-13-1.c: New test. * gcc.target/powerpc/darwin-abi-13-2.c: New test. * gcc.target/powerpc/darwin-structs-0.h: New test. --- gcc/config/rs6000/rs6000.cc | 3 +- .../gcc.target/powerpc/darwin-abi-13-0.c | 23 +++++++++++++++ .../gcc.target/powerpc/darwin-abi-13-1.c | 27 +++++++++++++++++ .../gcc.target/powerpc/darwin-abi-13-2.c | 27 +++++++++++++++++ .../gcc.target/powerpc/darwin-structs-0.h | 29 +++++++++++++++++++ 5 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-abi-13-0.c create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-abi-13-1.c create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-abi-13-2.c create mode 100644 gcc/testsuite/gcc.target/powerpc/darwin-structs-0.h