Re: [PATCH] i386: Always set cfun->machine->max_used_stack_alignment

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sat, Aug 4, 2018 at 3:42 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Aug 3, 2018 at 12:55 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> We should always set cfun->machine->max_used_stack_alignment if the
>> maximum stack slot alignment may be greater than 64 bits.
>>
>> Tested on i686 and x86-64.  OK for master and backport for GCC 8?
>
> Can you explain why 64 bits, and what this value represents? Is this
> value the same for 64bit and 32bit targets?
>
> Should crtl->max_used_stack_slot_alignment be compared to
> STACK_BOUNDARY or even MIN_STACK_BOUNDARY instead?

In this case, we don't need to realign the incoming stack since both
crtl->max_used_stack_slot_alignment and crtl->preferred_stack_boundary
are 128 bits.  We don't compute the largest alignment of stack slots to
keep stack frame properly aligned for it.  Normally it is OK.   But if
the largest alignment of stack slots > 64 bits and we don't keep stack
frame proper aligned, we will get segfault if aligned vector load/store
are used on these unaligned stack slots. My patch computes the
largest alignment of stack slots, which are actually used,  if the
largest alignment of stack slots allocated is > 64 bits, which is
the smallest alignment for misaligned load/store.

Here is the diff of before and after:

diff -up old/x.s new/x.s
--- old/x.s 2018-08-02 12:39:22.916400504 -0700
+++ new/x.s 2018-08-02 12:38:35.853729415 -0700
@@ -15,6 +15,7 @@ foo:
  movq %rsp, %rbp
  .cfi_def_cfa_register 6
  pushq %rbx
+ subq $8, %rsp  <<<<<<<<<<< Stack frame is properly aligned to 16 bytes.
  .cfi_offset 3, -24
  stosw
  movl 16(%rbp), %ecx
@@ -65,6 +66,7 @@ foo:
 .L9:
  xorl %r8d, d(%rip)
  movl %esi, %eax
+ popq %rdx
  popq %rbx
  popq %rbp
  .cfi_def_cfa 7, 8




-- 
H.J.