Stack alignment on Darwin (PR78444)

Message ID E457FE2C-FA13-48FE-ABD4-03B69B7C513C@sandoe.co.uk
State New
Headers show
Series
  • Stack alignment on Darwin (PR78444)
Related show

Commit Message

Iain Sandoe Aug. 15, 2018, 3:41 p.m.
Hi HJ,

I am trying to track down a misalignment of the stack on Darwin (pr78444).

In r163971 you applied this:

-- 
2.17.1

Comments

H.J. Lu Aug. 15, 2018, 3:57 p.m. | #1
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?


-- 
H.J.
Iain Sandoe Nov. 12, 2018, 9:03 a.m. | #2
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

Patch

--- 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;