* [pushed] Darwin, PPC: Fix struct layout with pragma pack [PR110044].
@ 2023-06-02 19:11 Iain Sandoe
2023-06-03 11:20 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Iain Sandoe @ 2023-06-02 19:11 UTC (permalink / raw)
To: gcc-patches; +Cc: segher, dje.gcc
@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)
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)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pushed] Darwin, PPC: Fix struct layout with pragma pack [PR110044].
2023-06-02 19:11 [pushed] Darwin, PPC: Fix struct layout with pragma pack [PR110044] Iain Sandoe
@ 2023-06-03 11:20 ` Richard Biener
2023-06-03 11:31 ` Iain Sandoe
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2023-06-03 11:20 UTC (permalink / raw)
To: iain; +Cc: gcc-patches, Iain Sandoe, segher, dje.gcc
> 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)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pushed] Darwin, PPC: Fix struct layout with pragma pack [PR110044].
2023-06-03 11:20 ` Richard Biener
@ 2023-06-03 11:31 ` Iain Sandoe
0 siblings, 0 replies; 3+ messages in thread
From: Iain Sandoe @ 2023-06-03 11:31 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Segher Boessenkool, David Edelsohn
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-03 11:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-02 19:11 [pushed] Darwin, PPC: Fix struct layout with pragma pack [PR110044] Iain Sandoe
2023-06-03 11:20 ` Richard Biener
2023-06-03 11:31 ` Iain Sandoe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).