| Message ID | E457FE2C-FA13-48FE-ABD4-03B69B7C513C@sandoe.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | Stack alignment on Darwin (PR78444) | expand |
On Wed, Aug 15, 2018 at 8:41 AM, Iain Sandoe <iain@sandoe.co.uk> wrote: > Hi HJ, > > I am trying to track down a misalignment of the stack on Darwin (pr78444). > > In r163971 you applied this: > > --- gcc/config/i386/darwin.h (revision 163970) > +++ gcc/config/i386/darwin.h (revision 163971) > @@ -79,7 +79,9 @@ > Failure to ensure this will lead to a crash in the system libraries > or dynamic loader. */ > #undef STACK_BOUNDARY > -#define STACK_BOUNDARY 128 > +#define STACK_BOUNDARY \ > + ((profile_flag || (TARGET_64BIT && ix86_abi == MS_ABI)) \ > + ? 128 : BITS_PER_WORD) > > #undef MAIN_STACK_BOUNDARY > #define MAIN_STACK_BOUNDARY 128 > @@ -91,7 +93,7 @@ > it's below the minimum. */ > #undef PREFERRED_STACK_BOUNDARY > #define PREFERRED_STACK_BOUNDARY \ > - MAX (STACK_BOUNDARY, ix86_preferred_stack_boundary) > + MAX (128, ix86_preferred_stack_boundary > > === > > I realise it’s a long time ago … > .. but, have you any pointer to the reasoning here or what problem was being solved? > (perhaps mail list discussion?) Please see PR target/36502, PR target/42313 and PR target/44651. > === > > Darwin’s 32b ABI mandates 128b alignment at functions calls: > > "The function calling conventions used in the IA-32 environment are the same as those used in the System V IA-32 ABI, with the following exceptions: > ■ Different rules for returning structures > ■ The stack is 16-byte aligned at the point of function calls > “ > > Darwin’s 64b ABI refers directly to the SysV document, which also mandates [section 3.2.2] 128b (or 256b when __m256 is passed). > > === > > The following patch resolves pr78444 - but it’s not clear if it’s a correct fix - or we should be looking for an alternate solution to whatever r193671 was intending to achieve. > > thanks, > Iain > > [PATCH] Fix for PR78444. > > maybe. > --- > gcc/config/i386/i386.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 163682bdff..405bfd082b 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -11530,6 +11530,15 @@ ix86_compute_frame_layout (void) > crtl->preferred_stack_boundary = 128; > crtl->stack_alignment_needed = 128; > } > + else if (TARGET_MACHO && crtl->preferred_stack_boundary < 128 > + && !crtl->is_leaf) > + { > + /* Darwin's ABI specifies 128b alignment for both 32 and > + 64 bit variants at call sites. So we apply this if the > + current function is not a leaf. */ > + crtl->preferred_stack_boundary = 128; > + crtl->stack_alignment_needed = 128; > + } > Can you change ix86_update_stack_boundary instead?
Appending Uros’ comments from a second thread on Stack alignment. Note that as per the new analysis in pr78444 this can affect other x86-64 targets too. > On 15 Aug 2018, at 16:57, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Aug 15, 2018 at 8:41 AM, Iain Sandoe <iain@sandoe.co.uk> wrote: >> Hi HJ, >> >> I am trying to track down a misalignment of the stack on Darwin (pr78444). >> >> In r163971 you applied this: >> >> --- gcc/config/i386/darwin.h (revision 163970) >> +++ gcc/config/i386/darwin.h (revision 163971) >> @@ -79,7 +79,9 @@ >> Failure to ensure this will lead to a crash in the system libraries >> or dynamic loader. */ >> #undef STACK_BOUNDARY >> -#define STACK_BOUNDARY 128 >> +#define STACK_BOUNDARY \ >> + ((profile_flag || (TARGET_64BIT && ix86_abi == MS_ABI)) \ >> + ? 128 : BITS_PER_WORD) >> >> #undef MAIN_STACK_BOUNDARY >> #define MAIN_STACK_BOUNDARY 128 >> @@ -91,7 +93,7 @@ >> it's below the minimum. */ >> #undef PREFERRED_STACK_BOUNDARY >> #define PREFERRED_STACK_BOUNDARY \ >> - MAX (STACK_BOUNDARY, ix86_preferred_stack_boundary) >> + MAX (128, ix86_preferred_stack_boundary >> >> === >> >> I realise it’s a long time ago … >> .. but, have you any pointer to the reasoning here or what problem was being solved? >> (perhaps mail list discussion?) > > Please see PR target/36502, PR target/42313 and PR target/44651. > >> === >> >> Darwin’s 32b ABI mandates 128b alignment at functions calls: >> >> "The function calling conventions used in the IA-32 environment are the same as those used in the System V IA-32 ABI, with the following exceptions: >> ■ Different rules for returning structures >> ■ The stack is 16-byte aligned at the point of function calls >> “ >> >> Darwin’s 64b ABI refers directly to the SysV document, which also mandates [section 3.2.2] 128b (or 256b when __m256 is passed). >> >> === >> >> The following patch resolves pr78444 - but it’s not clear if it’s a correct fix - or we should be looking for an alternate solution to whatever r193671 was intending to achieve. >> >> thanks, >> Iain >> >> [PATCH] Fix for PR78444. >> >> maybe. >> --- >> gcc/config/i386/i386.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 163682bdff..405bfd082b 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -11530,6 +11530,15 @@ ix86_compute_frame_layout (void) >> crtl->preferred_stack_boundary = 128; >> crtl->stack_alignment_needed = 128; >> } >> + else if (TARGET_MACHO && crtl->preferred_stack_boundary < 128 >> + && !crtl->is_leaf) >> + { >> + /* Darwin's ABI specifies 128b alignment for both 32 and >> + 64 bit variants at call sites. So we apply this if the >> + current function is not a leaf. */ >> + crtl->preferred_stack_boundary = 128; >> + crtl->stack_alignment_needed = 128; >> + } >> > > Can you change ix86_update_stack_boundary instead? Uros writes: "You can't use crtl->is_leaf in ix86_update_stack_boundary, since that function gets called from cfgexpand, while crtl->is_leaf is set only in IRA pass. I *think* the fix should be along the lines of TARGET_64BIT_MS_ABI fixup in ix86_compute_frame_layout (BTW: the existing fixup is strange by itself, since TARGET_64BIT_MS_ABI declares STACK_BOUNDARY to 128, and I can't see how leaf functions with crtl->preferred_stack_boundary < 128 survive "gcc_assert (preferred_alignment >= STACK_BOUNDARY / BITS_PER_UNIT);" a couple of lines below). So, I think that fixup you proposed in the patch is in the right direction. What happens if you add TARGET_MACHO to the existing fixup? “ I will test that suggestion and re-post - although, if the problem is not specific to Darwin, maybe a more general fix is needed. Iain
--- gcc/config/i386/darwin.h (revision 163970) +++ gcc/config/i386/darwin.h (revision 163971) @@ -79,7 +79,9 @@ Failure to ensure this will lead to a crash in the system libraries or dynamic loader. */ #undef STACK_BOUNDARY -#define STACK_BOUNDARY 128 +#define STACK_BOUNDARY \ + ((profile_flag || (TARGET_64BIT && ix86_abi == MS_ABI)) \ + ? 128 : BITS_PER_WORD) #undef MAIN_STACK_BOUNDARY #define MAIN_STACK_BOUNDARY 128 @@ -91,7 +93,7 @@ it's below the minimum. */ #undef PREFERRED_STACK_BOUNDARY #define PREFERRED_STACK_BOUNDARY \ - MAX (STACK_BOUNDARY, ix86_preferred_stack_boundary) + MAX (128, ix86_preferred_stack_boundary === I realise it’s a long time ago … .. but, have you any pointer to the reasoning here or what problem was being solved? (perhaps mail list discussion?) === Darwin’s 32b ABI mandates 128b alignment at functions calls: "The function calling conventions used in the IA-32 environment are the same as those used in the System V IA-32 ABI, with the following exceptions: ■ Different rules for returning structures ■ The stack is 16-byte aligned at the point of function calls “ Darwin’s 64b ABI refers directly to the SysV document, which also mandates [section 3.2.2] 128b (or 256b when __m256 is passed). === The following patch resolves pr78444 - but it’s not clear if it’s a correct fix - or we should be looking for an alternate solution to whatever r193671 was intending to achieve. thanks, Iain [PATCH] Fix for PR78444. maybe. --- gcc/config/i386/i386.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 163682bdff..405bfd082b 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -11530,6 +11530,15 @@ ix86_compute_frame_layout (void) crtl->preferred_stack_boundary = 128; crtl->stack_alignment_needed = 128; } + else if (TARGET_MACHO && crtl->preferred_stack_boundary < 128 + && !crtl->is_leaf) + { + /* Darwin's ABI specifies 128b alignment for both 32 and + 64 bit variants at call sites. So we apply this if the + current function is not a leaf. */ + crtl->preferred_stack_boundary = 128; + crtl->stack_alignment_needed = 128; + } stack_alignment_needed = crtl->stack_alignment_needed / BITS_PER_UNIT; preferred_alignment = crtl->preferred_stack_boundary / BITS_PER_UNIT;