[PATCH,rs6000] fix interrupt safety issue on E500 targets
Alan Modra
amodra@bigpond.net.au
Fri Oct 5 08:30:00 GMT 2007
On Thu, Oct 04, 2007 at 11:12:53AM +0930, Alan Modra wrote:
> On Wed, Oct 03, 2007 at 08:41:58AM -0700, Andrew Pinski wrote:
> > I think this is needed for all sysv based abis and not just SPE.
>
> No, it shouldn't be needed. Plain powerpc-linux will restore regs
> using either sp or fp relative memory addressing. gcc shouldn't move
> the sp adjust above use of sp, which is why we currently only emit the
> stack tie when frame_reg_rtx != sp_reg_rtx. The problem appears to be
> specific to spe, which uses yet another register rtx to restore regs.
That wasn't a really clear explanation. What I should have said was:
The scheduler will not move the stack adjustment (via sp_reg_rtx) past
register loads using a MEM with sp_reg_rtx in the address. We only
need the stack tie when we reference MEMs without sp_reg_rtx in their
addresses. If you examine rs6000_emit_epilogue you'll see that all of
the register restores use frame_reg_rtx in the MEM address, except for
some SPE code. frame_reg_rtx is set to sp_reg_rtx at the start of
this function, and changed in only one place, for ABI_V4. Thus, prior
to the addition of SPE support, it was correct to test
frame_reg_rtx != sp_reg_rtx to determine whether a stack tie was
needed.
So, another fix for the SPE problem is to use frame_reg_rtx in the SPE
support code instead of inventing another rtx for r11. That's what I
set out to do in the following patch, but then noticed a few other
problems..
Firstly, there's a glaring error dating back to when Eric moved the
altivec register restore before the stack update. Before we pop the
stack frame, sp_offset can't possibly depend on the size of the stack
frame, only on whether we have a frame or not.
Secondly, the SPE stack restore is wrong when
use_backchain_to_restore_sp and spe_regs_addressable_via_sp. In that
case, r11 is not set by the SPE register restore code so remains
pointing to the top of the stack frame.
* config/rs6000/rs6000.c (rs6000_emit_epilogue): Correct
altivec sp_offset. Rearrange sp_offset assignments to
correspond to stack adjustments. Use frame_reg_rtx for
SPE register restores. Correct SPE stack adjustment.
A powerpc-linux bootstrap/regtest is still in progress. I don't have
access to SPE hardware to properly test the SPE changes, so would
someone mind regression testing this for me on SPE?
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 128777)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -15998,8 +16014,8 @@ rs6000_emit_epilogue (int sibcall)
return;
}
- /* Set sp_offset based on the stack push from the prologue. */
- if (info->total_size < 32767)
+ /* frame_reg_rtx + sp_offset points to the top of this stack frame. */
+ if (info->push_p)
sp_offset = info->total_size;
/* Restore AltiVec registers if needed. */
@@ -16041,8 +16057,6 @@ rs6000_emit_epilogue (int sibcall)
emit_insn (generate_set_vrsave (reg, info, 1));
}
- sp_offset = 0;
-
/* If we have a frame pointer, a call to alloca, or a large stack
frame, restore the old stack pointer using the backchain. Otherwise,
we know what size to update it with. */
@@ -16055,20 +16069,18 @@ rs6000_emit_epilogue (int sibcall)
emit_move_insn (frame_reg_rtx,
gen_rtx_MEM (Pmode, sp_reg_rtx));
+ sp_offset = 0;
}
- else if (info->push_p)
+ else if (info->push_p
+ && DEFAULT_ABI != ABI_V4
+ && !current_function_calls_eh_return)
{
- if (DEFAULT_ABI == ABI_V4
- || current_function_calls_eh_return)
- sp_offset = info->total_size;
- else
- {
- emit_insn (TARGET_32BIT
- ? gen_addsi3 (sp_reg_rtx, sp_reg_rtx,
- GEN_INT (info->total_size))
- : gen_adddi3 (sp_reg_rtx, sp_reg_rtx,
- GEN_INT (info->total_size)));
- }
+ emit_insn (TARGET_32BIT
+ ? gen_addsi3 (sp_reg_rtx, sp_reg_rtx,
+ GEN_INT (info->total_size))
+ : gen_adddi3 (sp_reg_rtx, sp_reg_rtx,
+ GEN_INT (info->total_size)));
+ sp_offset = 0;
}
/* Get the old lr if we saved it. */
@@ -16150,7 +16162,6 @@ rs6000_emit_epilogue (int sibcall)
&& info->spe_64bit_regs_used != 0
&& info->first_gp_reg_save != 32)
{
- rtx spe_save_area_ptr;
/* Determine whether we can address all of the registers that need
to be saved with an offset from the stack pointer that fits in
the small const field for SPE memory instructions. */
@@ -16160,20 +16171,21 @@ rs6000_emit_epilogue (int sibcall)
int spe_offset;
if (spe_regs_addressable_via_sp)
- {
- spe_save_area_ptr = frame_reg_rtx;
- spe_offset = info->spe_gp_save_offset + sp_offset;
- }
+ spe_offset = info->spe_gp_save_offset + sp_offset;
else
{
+ rtx old_frame_reg_rtx = frame_reg_rtx;
/* Make r11 point to the start of the SPE save area. We worried about
not clobbering it when we were saving registers in the prologue.
There's no need to worry here because the static chain is passed
anew to every function. */
- spe_save_area_ptr = gen_rtx_REG (Pmode, 11);
-
- emit_insn (gen_addsi3 (spe_save_area_ptr, frame_reg_rtx,
+ if (frame_reg_rtx == sp_reg_rtx)
+ frame_reg_rtx = gen_rtx_REG (Pmode, 11);
+ emit_insn (gen_addsi3 (frame_reg_rtx, old_frame_reg_rtx,
GEN_INT (info->spe_gp_save_offset + sp_offset)));
+ /* Keep the invariant that frame_reg_rtx + sp_offset points
+ at the top of the stack frame. */
+ sp_offset = -info->spe_gp_save_offset;
spe_offset = 0;
}
@@ -16188,7 +16200,7 @@ rs6000_emit_epilogue (int sibcall)
gcc_assert (SPE_CONST_OFFSET_OK (spe_offset + reg_size * i));
offset = GEN_INT (spe_offset + reg_size * i);
- addr = gen_rtx_PLUS (Pmode, spe_save_area_ptr, offset);
+ addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, offset);
mem = gen_rtx_MEM (V2SImode, addr);
emit_move_insn (gen_rtx_REG (reg_mode, info->first_gp_reg_save + i),
@@ -16280,11 +16292,9 @@ rs6000_emit_epilogue (int sibcall)
/* This blockage is needed so that sched doesn't decide to move
the sp change before the register restores. */
rs6000_emit_stack_tie ();
- if (TARGET_SPE_ABI
- && info->spe_64bit_regs_used != 0
- && info->first_gp_reg_save != 32)
- emit_insn (gen_addsi3 (sp_reg_rtx, gen_rtx_REG (Pmode, 11),
- GEN_INT (-(info->spe_gp_save_offset + sp_offset))));
+ if (sp_offset != 0)
+ emit_insn (gen_addsi3 (sp_reg_rtx, frame_reg_rtx,
+ GEN_INT (sp_offset)));
else
emit_move_insn (sp_reg_rtx, frame_reg_rtx);
}
--
Alan Modra
Australia Development Lab, IBM
More information about the Gcc-patches
mailing list