Lots of bugs here. Most of them in TARGET_SPE_ABI code, but some also
for other ABIs.
1) Marking an instruction setting up r11 for use by _save64gpr_* as
frame-related results in r11 being set as the cfa for the rest of the
function. That's bad when r11 gets used for other things later.
Even worse, the offset given was wrong. This bug was likely the
primary cause for the segfault reported in the PR.
2) RTL emitted to describe the _save64gpr_* used wrong offsets.
Harmless at the moment, I think, but still incorrect. Ditto for the
epilogue _rest64gpr_* RTL. eg. _save64gpr_24 rtl said the store for
r24 happened at 0(11), but see e500crtsav64gpr.S.
3) The .cfi_offset values given for the SPE _save64gpr_* reg saves
were wrong.
4) sp_offset was not set correctly in a number of places where
out-of-line save and restore functions were used, possibly leading to
incorrect code later in the prologue/epilogue. For instance in this
code:
+ emit_insn (gen_add3_insn (src_reg, frame_reg_rtx,
GEN_INT (sp_offset - info->fp_size)));
- if (REGNO (frame_reg_rtx) == 11)
- sp_offset += info->fp_size;
+ if (REGNO (frame_reg_rtx) == REGNO (src_reg))
+ sp_offset = info->fp_size;
src_reg can be r12, so testing for r11 as frame_reg_rtx is wrong. You
want to update sp_offset when frame_reg_rtx changes. Also, adding
info->fp_size to sp_offset is obviously wrong when you've just added
(sp_offset - info->fp_size) to frame_reg_rtx. Clearly you want to
subtract (sp_offset - info->fp_size) from sp_offset, to keep the
invariant that frame_reg_rtx + sp_offset points to top of frame.
Really, sp_offset should be renamed to frame_offset.
5) The wrong register mode was used in a number of places for address
calculations. A nitpick, but reg_mode is for data, Pmode for addresses.
6) The epilogue _rest64gpr_* r11 wasn't always set up.
7) Not fixed by this patch, but many instances of rs6000_frame_related
and emit_frame_save pass frame_ptr_rtx for the reg to replace with
sp+offset in the cfi expression, when frame_ptr_rtx is not the reg
being used in the memory access. That means we're relying on the
generic dwarf code tracking registers for us. Which does seem to
work. However, it's confusing to have calls to rs6000_frame_related
that really don't do anything.
Bootstrapped and regression tested powerpc-linux, with standard
BOOT_CFLAGS, with BOOT_CFLAGS="-g -Os" (tends to test just out-of-line
restore), and with BOOT_CFLAGS="-g -Os -mno-multiple" (test
out-of-line save and restore). The PR reporter is running SPE
bootstrap and regression tests. Incidentally, I had to use
--disable-werror as we run into some bogus warnings when trying to
bootstrap with -Os.
gcc/calls.c: In function 'rtx_def* expand_call(tree, rtx, int)':
gcc/calls.c:1098:52: warning: 'base' may be used uninitialized in this function
[-Wmaybe-uninitialized]
gcc/calls.c:1079:9: note: 'base' was declared here
OK to apply? gcc-4.6 branch too?
PR target/50906
* config/rs6000/rs6000.c (rs6000_emit_prologue <TARGET_SPE_ABI>):
Do not mark r11 setup as frame-related. Pass correct offset to
rs6000_emit_savres_rtx. Correct out-of-line rs6000_frame_related
arguments. Correct sp_offset. Remove "offset" fudge from
in-line rs6000_frame_related call. Rename misleading variable.
Fix comments and whitespace. Tidy some expressions.
(rs6000_emit_epilogue <TARGET_SPE_ABI>): Always set frame_reg_rtx
to r11 in out-of-line case. Correct sp_offset. Pass correct
offset to rs6000_emit_savres_rtx. Rename misleading variable.
Fix comments and whitespace. Tidy some expressions.
(rs6000_emit_epilogue <non-TARGET_SPE_ABI>): Add sp_offset
adjustment when !saving_GPRs_inline. Correct register mode
used in address calcs.
(rs6000_emit_epilogue <non-TARGET_SPE_ABI>): Similarly when
!restoring_GPRs_inline.
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 180867)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -20089,56 +20115,52 @@ rs6000_emit_prologue (void)
{
int i;
rtx spe_save_area_ptr;
-
+ int save_ptr_to_sp;
+ int ool_adjust = 0;
+
/* Determine whether we can address all of the registers that need
- to be saved with an offset from the stack pointer that fits in
+ to be saved with an offset from frame_reg_rtx that fits in
the small const field for SPE memory instructions. */
- int spe_regs_addressable_via_sp
- = (SPE_CONST_OFFSET_OK(info->spe_gp_save_offset + sp_offset
- + (32 - info->first_gp_reg_save - 1) * reg_size)
+ int spe_regs_addressable
+ = (SPE_CONST_OFFSET_OK (info->spe_gp_save_offset + sp_offset
+ + reg_size * (32 - info->first_gp_reg_save - 1))
&& saving_GPRs_inline);
int spe_offset;
-
- if (spe_regs_addressable_via_sp)
+
+ if (spe_regs_addressable)
{
spe_save_area_ptr = frame_reg_rtx;
+ save_ptr_to_sp = info->total_size - sp_offset;
spe_offset = info->spe_gp_save_offset + sp_offset;
}
else
{
/* Make r11 point to the start of the SPE save area. We need
to be careful here if r11 is holding the static chain. If
- it is, then temporarily save it in r0. We would use r0 as
- our base register here, but using r0 as a base register in
- loads and stores means something different from what we
- would like. */
- int ool_adjust = (saving_GPRs_inline
- ? 0
- : (info->first_gp_reg_save
- - (FIRST_SAVRES_REGISTER+1))*8);
- HOST_WIDE_INT offset = (info->spe_gp_save_offset
- + sp_offset - ool_adjust);
+ it is, then temporarily save it in r0. */
+ int offset;
+
+ if (!saving_GPRs_inline)
+ ool_adjust = 8 * (info->first_gp_reg_save
+ - (FIRST_SAVRES_REGISTER + 1));
+ offset = info->spe_gp_save_offset + sp_offset - ool_adjust;
+ spe_save_area_ptr = gen_rtx_REG (Pmode, 11);
+ save_ptr_to_sp = info->total_size - sp_offset + offset;
+ spe_offset = 0;
if (using_static_chain_p)
{
rtx r0 = gen_rtx_REG (Pmode, 0);
gcc_assert (info->first_gp_reg_save > 11);
-
- emit_move_insn (r0, gen_rtx_REG (Pmode, 11));
+
+ emit_move_insn (r0, spe_save_area_ptr);
}
-
- spe_save_area_ptr = gen_rtx_REG (Pmode, 11);
- insn = emit_insn (gen_addsi3 (spe_save_area_ptr,
- frame_reg_rtx,
- GEN_INT (offset)));
- /* We need to make sure the move to r11 gets noted for
- properly outputting unwind information. */
- if (!saving_GPRs_inline)
- rs6000_frame_related (insn, frame_reg_rtx, offset,
- NULL_RTX, NULL_RTX);
- spe_offset = 0;
+ emit_insn (gen_addsi3 (spe_save_area_ptr,
+ frame_reg_rtx, GEN_INT (offset)));
+ if (REGNO (frame_reg_rtx) == 11)
+ sp_offset = -info->spe_gp_save_offset + ool_adjust;
}
-
+
if (saving_GPRs_inline)
{
for (i = 0; i < 32 - info->first_gp_reg_save; i++)
@@ -20150,58 +20172,65 @@ rs6000_emit_prologue (void)
/* We're doing all this to ensure that the offset fits into
the immediate offset of 'evstdd'. */
gcc_assert (SPE_CONST_OFFSET_OK (reg_size * i + spe_offset));
-
+
offset = GEN_INT (reg_size * i + spe_offset);
addr = gen_rtx_PLUS (Pmode, spe_save_area_ptr, offset);
mem = gen_rtx_MEM (V2SImode, addr);
-
+
insn = emit_move_insn (mem, reg);
-
- rs6000_frame_related (insn, spe_save_area_ptr,
- info->spe_gp_save_offset
- + sp_offset + reg_size * i,
- offset, const0_rtx);
+
+ rs6000_frame_related (insn,
+ spe_save_area_ptr, save_ptr_to_sp,
+ NULL_RTX, NULL_RTX);
}
}
else
{
- insn = rs6000_emit_savres_rtx (info, gen_rtx_REG (Pmode, 11),
- 0, reg_mode,
+ insn = rs6000_emit_savres_rtx (info, spe_save_area_ptr,
+ ool_adjust, reg_mode,
/*savep=*/true, /*gpr=*/true,
/*lr=*/false);
- rs6000_frame_related (insn, frame_ptr_rtx, info->total_size,
+
+ rs6000_frame_related (insn, spe_save_area_ptr, save_ptr_to_sp,
NULL_RTX, NULL_RTX);
}
-
-
+
/* Move the static chain pointer back. */
- if (using_static_chain_p && !spe_regs_addressable_via_sp)
- emit_move_insn (gen_rtx_REG (Pmode, 11), gen_rtx_REG (Pmode, 0));
+ if (using_static_chain_p && !spe_regs_addressable)
+ emit_move_insn (spe_save_area_ptr, gen_rtx_REG (Pmode, 0));
}
else if (!WORLD_SAVE_P (info) && !saving_GPRs_inline)
{
if (DEFAULT_ABI == ABI_DARWIN)
{
- rtx dest_reg = gen_rtx_REG (reg_mode, 11);
+ rtx dest_reg = gen_rtx_REG (Pmode, 11);
if (info->first_fp_reg_save == 64)
- /* we only need a copy, no fprs were saved. */
- emit_move_insn (dest_reg, frame_reg_rtx);
+ {
+ /* we only need a copy, no fprs were saved. */
+ if (dest_reg != frame_reg_rtx)
+ emit_move_insn (dest_reg, frame_reg_rtx);
+ }
else
{
- rtx offset = GEN_INT (sp_offset
- + (-8 * (64-info->first_fp_reg_save)));
+ int save_off = 8 * (64 - info->first_fp_reg_save);
+ rtx offset = GEN_INT (sp_offset - save_off);
+
+ if (REGNO (dest_reg) == REGNO (frame_reg_rtx))
+ sp_offset = save_off;
emit_insn (gen_add3_insn (dest_reg, frame_reg_rtx, offset));
}
}
/* Need to adjust r11 (r12) if we saved any FPRs. */
else if (info->first_fp_reg_save != 64)
- {
- rtx dest_reg = gen_rtx_REG (reg_mode, DEFAULT_ABI == ABI_AIX
- ? 12 : 11);
- rtx offset = GEN_INT (sp_offset
- + (-8 * (64-info->first_fp_reg_save)));
+ {
+ rtx dest_reg = gen_rtx_REG (Pmode, DEFAULT_ABI == ABI_AIX ? 12 : 11);
+ int save_off = 8 * (64 - info->first_fp_reg_save);
+ rtx offset = GEN_INT (sp_offset - save_off);
+
+ if (REGNO (dest_reg) == REGNO (frame_reg_rtx))
+ sp_offset = save_off;
emit_insn (gen_add3_insn (dest_reg, frame_reg_rtx, offset));
- }
+ }
insn = rs6000_emit_savres_rtx (info, frame_reg_rtx,
info->gp_save_offset + sp_offset,
@@ -21115,40 +21162,39 @@ rs6000_emit_epilogue (int sibcall)
&& info->first_gp_reg_save != 32)
{
/* 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. */
- int spe_regs_addressable_via_sp
- = (SPE_CONST_OFFSET_OK(info->spe_gp_save_offset + sp_offset
- + (32 - info->first_gp_reg_save - 1) * reg_size)
+ to be saved with an offset from frame_reg_rtx that fits in
+ the small const field for SPE memory instructions. */
+ int spe_regs_addressable
+ = (SPE_CONST_OFFSET_OK (info->spe_gp_save_offset + sp_offset
+ + reg_size * (32 - info->first_gp_reg_save - 1))
&& restoring_GPRs_inline);
int spe_offset;
+ int ool_adjust = 0;
- if (spe_regs_addressable_via_sp)
+ if (spe_regs_addressable)
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. */
- int ool_adjust = (restoring_GPRs_inline
- ? 0
- : (info->first_gp_reg_save
- - (FIRST_SAVRES_REGISTER + 1)) * 8);
-
- 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,
+ /* 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. */
+
+ if (!restoring_GPRs_inline)
+ ool_adjust = 8 * (info->first_gp_reg_save
+ - (FIRST_SAVRES_REGISTER + 1));
+ 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
- ool_adjust)));
/* Keep the invariant that frame_reg_rtx + sp_offset points
at the top of the stack frame. */
- sp_offset = -info->spe_gp_save_offset;
+ sp_offset = -info->spe_gp_save_offset + ool_adjust;
- spe_offset = 0;
- }
+ spe_offset = 0;
+ }
if (restoring_GPRs_inline)
{
@@ -21170,8 +21216,8 @@ rs6000_emit_epilogue (int sibcall)
}
}
else
- rs6000_emit_savres_rtx (info, gen_rtx_REG (Pmode, 11),
- 0, reg_mode,
+ rs6000_emit_savres_rtx (info, frame_reg_rtx,
+ ool_adjust, reg_mode,
/*savep=*/false, /*gpr=*/true,
/*lr=*/true);
}
@@ -21184,22 +21230,22 @@ rs6000_emit_epilogue (int sibcall)
if (can_use_exit)
{
rs6000_emit_stack_reset (info, sp_reg_rtx, frame_reg_rtx,
- sp_offset, can_use_exit);
+ sp_offset, can_use_exit);
if (DEFAULT_ABI == ABI_DARWIN)
/* we only need a copy, no fprs were saved. */
- emit_move_insn (gen_rtx_REG (reg_mode, 11), frame_reg_rtx);
+ emit_move_insn (gen_rtx_REG (Pmode, 11), frame_reg_rtx);
if (info->cr_save_p)
rs6000_restore_saved_cr (cr_save_reg, using_mtcr_multiple);
}
else
{
- emit_insn (gen_add3_insn (gen_rtx_REG (Pmode, DEFAULT_ABI == ABI_AIX
- ? 12 : 11),
- frame_reg_rtx,
+ rtx src_reg = gen_rtx_REG (Pmode, DEFAULT_ABI == ABI_AIX ? 12 : 11);
+
+ emit_insn (gen_add3_insn (src_reg, frame_reg_rtx,
GEN_INT (sp_offset - info->fp_size)));
- if (REGNO (frame_reg_rtx) == 11)
- sp_offset += info->fp_size;
+ if (REGNO (frame_reg_rtx) == REGNO (src_reg))
+ sp_offset = info->fp_size;
}
rs6000_emit_savres_rtx (info, frame_reg_rtx,
--
Alan Modra
Australia Development Lab, IBM
