On 07/26/2017 05:29 PM, Wilco Dijkstra wrote:
> Jeff Law wrote:
>
>> + if (required_align > MAX_SUPPORTED_STACK_ALIGNMENT)
>> + {
>> + extra = (required_align - MAX_SUPPORTED_STACK_ALIGNMENT)
>> + / BITS_PER_UNIT;
>> + size = plus_constant (Pmode, size, extra);
>> + size = force_operand (size, NULL_RTX);
>>
>> - if (extra && size_align > BITS_PER_UNIT)
>> - size_align = BITS_PER_UNIT;
>> + if (flag_stack_usage_info && pstack_usage_size)
>> + *pstack_usage_size += extra;
>> + }
>
>> So it's unclear to me why you increase the size iff REQUIRED_ALIGN is
>> greater than MAX_SUPPORTED_STACK_ALIGNMENT.
>>
>> ISTM the safe assumption we can make in this code is that the stack is
>> aligned to STACK_BOUNDARY. If so, isn't the right test
>>
>> (required_align > STACK_BOUNDARY)?
>>
>> And once we decide to make an adjustment, isn't the right adjustment to
>> make (required_align - STACK_BOUNDARY) / BITS_PER_UNIT?
>>
>> I feel like I must be missing something really important here.
>
> Yes I think you're right that if STACK_BOUNDARY is the guaranteed stack
> alignment, it should check against STACK_BOUNDARY indeed.
Seems that way to me.
>
> But then the check size_align % MAX_SUPPORTED_STACK_ALIGNMENT != 0
> seems wrong too given that round_push uses a different alignment to align to.
I had started to dig into the history of this code, but just didn't have
the time to do so fully before needing to leave for the day. To some
degree I was hoping you knew the rationale behind the test against
MAX_SUPPORTED_STACK_ALIGNMENT and I wouldn't have to do a ton of digging :-)
Jeff
