| Message ID | 4E633E19.3040702@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
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? 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. >
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. >>
On Sun, Sep 4, 2011 at 6:44 AM, Tom de Vries <vries@codesourcery.com> wrote: > 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. > There are 2 aspects in stack realignments: 1. Support stack variables with any alignment. 2. Support register spill with alignment > incoming stack alignment. You can test #2 on Sandy Bridge Linux by configuring gcc with --with-arch=corei7-avx. H.J.
Index: gcc/explow.c =================================================================== --- gcc/explow.c (revision 178145) +++ gcc/explow.c (working copy) @@ -1062,6 +1062,20 @@ emit_stack_restore (enum save_level save /* The default is that we use a move insn. */ rtx (*fcn) (rtx, rtx) = gen_move_insn; + /* If stack_realign_drap, the x86 backend emits a prologue that aligns both + STACK_POINTER and HARD_FRAME_POINTER. + If stack_realign_fp, the x86 backend emits a prologue that aligns only + STACK_POINTER. This renders the HARD_FRAME_POINTER unusable for accessing + aligned variables, which is reflected in ix86_can_eliminate. + We normally still have the realigned STACK_POINTER that we can use. + But if there is a stack restore still present at reload, it can trigger + mark_not_eliminable for the STACK_POINTER, leaving no way to eliminate + FRAME_POINTER into a hard reg. + To prevent this situation, we force need_drap if we emit a stack + restore. */ + if (SUPPORTS_STACK_ALIGNMENT) + crtl->need_drap = true; + /* See if this machine has anything special to do for this kind of save. */ switch (save_level) {