On Sat, Aug 4, 2018 at 3:59 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Sat, Aug 4, 2018 at 3:42 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> On Fri, Aug 3, 2018 at 12:55 AM, H.J. Lu <hongjiu...@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.
Does > 64 bits also hold for 32 bit targets, and -mabi=ms? I think
that we need to compare to STACK_BOUNDARY instead:
--cut here--
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 263307)
+++ config/i386/i386.c (working copy)
@@ -13281,8 +13281,7 @@
recompute_frame_layout_p = true;
}
}
- else if (crtl->max_used_stack_slot_alignment
- > crtl->preferred_stack_boundary)
+ else if (crtl->max_used_stack_slot_alignment > STACK_BOUNDARY)
{
/* We don't need to realign stack. But we still need to keep
stack frame properly aligned to satisfy the largest alignment
--cut here--
(The testcase works OK with -mabi=ms, which somehow suggests that we
don't need realignment in this case).
Uros.
