Re: [PATCH, PR50251] set DRAP type stack realignment for stack_restore

[Tom de Vries <vries at codesourcery dot com>]

On 09/04/2011 11:10 AM, Richard Guenther wrote:
> On Sun, Sep 4, 2011 at 11:00 AM, Tom de Vries <vries@codesourcery.com> wrote:
>> Hi,
>>
>> this patch fixes PR50251, which was caused by r178353.
>>
>> The patch was bootstrapped and reg-tested on i686 and x86_64.
>> On i686, the test-cases reported failing in PR50251 pass again.
>>
>> The patch selects the DRAP type stack realignment method in case a stack_restore
>> is used. If that is not done, the presence of the stack_restore at reload leaves
>> FRAME_POINTER without an elimination rule for i386 port.
>>
>> OK for trunk?
> 
> Shouldn't we eventually simply set cfun->calls_alloca when expanding either
> stack save or restore?  Similar to how it is set from
> allocate_dynamic_stack_space?
> I'm not sure we encountered the dead stack save/restore pair before
> this new folding,
> so, just to make other targets not confused about them as well?

Setting cfun->calls_alloca will work as well, but doesn't seem necessary to me.
AFAIU, since other targets don't define MAX_STACK_ALIGNMENT, they don't need to
do a realign themselves. If I disable MAX_STACK_ALIGNMENT on i386, the
middle-end handles the realign and the stack_restore causes no problems. So to
me this seems a problem with the i386-specific implementation of realignment.

I'm more worried about other similar cases not working for i386 than about other
ports. Another, more involved way to fix this, would be in reload to:
- calculate which registers we cannot use for elimination (which we already do)
- pass that as context to target.can_eliminate. The i386 target can then
  fulfill its requirement to be able to eliminate the frame pointer by
  setting need_drap if the stack pointer is not available.
I think this way we could remove most if not all of the
'crtl->need_drap = true' snippets, and have a completer solution and perhaps
also more optimal.

Is this simple crtl->need_drap fix ok for now, or should I start testing the
cfun->calls_alloca fix?

Thanks,
- Tom

> 
> Thanks,
> Richard.
> 
>> Thanks,
>> - Tom
>>
>>
>> 2011-09-03  Tom de Vries  <tom@codesourcery.com>
>>
>>            * explow.c (emit_stack_restore): Set crtl->need_drap if
>>            stack_restore is emitted.
>>