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.to...@gmail.com> wrote:
> 
> On Wed, Aug 15, 2018 at 8:41 AM, Iain Sandoe <i...@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

Reply via email to