This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: patch for elimination to SP when it is changed in RTL (PR57293)
- From: Vladimir Makarov <vmakarov at redhat dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Earnshaw <rearnsha at arm dot com>
- Date: Mon, 02 Dec 2013 18:44:14 -0500
- Subject: Re: patch for elimination to SP when it is changed in RTL (PR57293)
- Authentication-results: sourceware.org; auth=none
- References: <5297BF8E dot 3070100 at redhat dot com> <20131201125722 dot GA3205 at arm dot com>
On 12/1/2013, 7:57 AM, James Greenhalgh wrote:
On Thu, Nov 28, 2013 at 10:11:26PM +0000, Vladimir Makarov wrote:
Committed as rev. 205498.
2013-11-28 Vladimir Makarov<vmakarov@redhat.com>
PR target/57293
* ira.h (ira_setup_eliminable_regset): Remove parameter.
* ira.c (ira_setup_eliminable_regset): Ditto. Add
SUPPORTS_STACK_ALIGNMENT for crtl->stack_realign_needed.
Don't call lra_init_elimination.
(ira): Call ira_setup_eliminable_regset without arguments.
* loop-invariant.c (calculate_loop_reg_pressure): Remove argument
from ira_setup_eliminable_regset call.
* gcse.c (calculate_bb_reg_pressure): Ditto.
* haifa-sched.c (sched_init): Ditto.
* lra.h (lra_init_elimination): Remove the prototype.
* lra-int.h (lra_insn_recog_data): New member sp_offset. Move
used_insn_alternative upper.
(lra_eliminate_regs_1): Add one more parameter.
(lra-eliminate): Ditto.
* lra.c (lra_invalidate_insn_data): Set sp_offset.
(setup_sp_offset): New.
(lra_process_new_insns): Call setup_sp_offset.
(lra): Add argument to lra_eliminate calls.
* lra-constraints.c (get_equiv_substitution): Rename to get_equiv.
(get_equiv_with_elimination): New.
(process_addr_reg): Call get_equiv_with_elimination instead of
get_equiv_substitution.
(equiv_address_substitution): Ditto.
(loc_equivalence_change_p): Ditto.
(loc_equivalence_callback, lra_constraints): Ditto.
(curr_insn_transform): Ditto. Print the sp offset
(process_alt_operands): Prevent stack pointer reloads.
(lra_constraints): Remove one argument from lra_eliminate call.
Move it up. Mark used hard regs bfore it. Use
get_equiv_with_elimination instead of get_equiv_substitution.
* lra-eliminations.c (lra_eliminate_regs_1): Add parameter and
assert for param values combination. Use sp offset. Add argument
to lra_eliminate_regs_1 calls.
(lra_eliminate_regs): Add argument to lra_eliminate_regs_1 call.
(curr_sp_change): New static var.
(mark_not_eliminable): Add parameter. Update curr_sp_change.
Don't prevent elimination to sp if we can calculate its change.
Pass the argument to mark_not_eliminable calls.
(eliminate_regs_in_insn): Add a parameter. Use sp offset. Add
argument to lra_eliminate_regs_1 call.
(update_reg_eliminate): Move calculation of hard regs for spill
lower. Switch off lra_in_progress temporarily to generate regs
involved into elimination.
(lra_init_elimination): Rename to init_elimination. Make it
static. Set up insn sp offset, check the offsets at the end of
BBs.
(process_insn_for_elimination): Add parameter. Pass its value to
eliminate_regs_in_insn.
(lra_eliminate): : Add parameter. Pass its value to
process_insn_for_elimination. Add assert for param values
combination. Call init_elimination. Don't update offsets in
equivalence substitutions.
* lra-spills.c (assign_mem_slot): Don't call lra_eliminate_regs_1
for created stack slot.
(remove_pseudos): Call lra_eliminate_regs_1 before changing memory
onto stack slot.
2013-11-28 Vladimir Makarov<vmakarov@redhat.com>
PR target/57293
* gcc.target/i386/pr57293.c: New.
Hi Vlad,
This patch seems to cause some problems for AArch64. I see an assert
triggering when building libgloss:
/work/gcc-clean/build-aarch64-none-elf/obj/gcc1/gcc/xgcc -B/work/gcc-clean/build-aarch64-none-elf/obj/gcc1/gcc/ -B/work/gcc-clean/build-aarch64-none-elf/obj/binutils/aarch64-none-elf/newlib/ -isystem /work/gcc-clean/build-aarch64-none-elf/obj/binutils/aarch64-none-elf/newlib/targ-include -isystem /work/gcc-clean/src/binutils/newlib/libc/include -B/work/gcc-clean/build-aarch64-none-elf/obj/binutils/aarch64-none-elf/libgloss/aarch64 -L/work/gcc-clean/build-aarch64-none-elf/obj/binutils/aarch64-none-elf/libgloss/libnosys -L/work/gcc-clean/src/binutils/libgloss/aarch64 -L/work/gcc-clean/build-aarch64-none-elf/obj/binutils/./ld -O2 -g -O2 -g -I. -I/work/gcc-clean/src/binutils/libgloss/aarch64/.. -DARM_RDI_MONITOR -o rdimon-_exit.o -c /work/gcc-clean/src/binutils/libgloss/aarch64/_exit.c
/work/gcc-clean/src/binutils/libgloss/aarch64/_exit.c: In function '_exit':
/work/gcc-clean/src/binutils/libgloss/aarch64/_exit.c:41:1: internal compiler error: in update_reg_eliminate, at lra-eliminations.c:1157
}
^
First of all, it is a bad situation for code performance when IRA
decides that it can use frame pointer for allocation, and after that
LRA/reload decides that frame pointer can not be used and spills all
pseudos assigned to FP. The generated code will be much worse than
one generated if we decided not to use FP from the IRA start.
Therefore I decided to put an assert for checking the situation. It
was triggered by your case. I think the problem in the following
code.
aarch64_can_eliminate (const int from, const int to)
{
if (frame_pointer_needed)
...
else
{
...
if (from == FRAME_POINTER_REGNUM && to == STACK_POINTER_REGNUM
&& df_regs_ever_live_p (LR_REGNUM)
&& faked_omit_frame_pointer)
return false;
}
return true;
}
and
static bool
aarch64_frame_pointer_required (void)
{
...
if (flag_omit_frame_pointer && !faked_omit_frame_pointer)
return false;
else if (flag_omit_leaf_frame_pointer)
return !crtl->is_leaf;
return true;
}
IRA calls hook frame_pointer_required and it returns false. After
that LRA calls can_eliminate hook and it returns false which means
that fp can not be used for allocation and we should spill all pseudos
assigned to it.
So the problem can be solved by modifying frame_pointer_required
hook.
I also don't like
if (from == FRAME_POINTER_REGNUM && to == STACK_POINTER_REGNUM
&& df_regs_ever_live_p (LR_REGNUM)
&& faked_omit_frame_pointer)
return false;
It means we cannot eliminate FP by SP but we still can eliminate AP
by SP, for example. I think it is wrong.
So the following patch solves the problem and improves generated code.
If somebody with the rights approves, I can commit it tomorrow.
2013-12-02 Vladimir Makarov <vmakarov@redhat.com>
* config/aarch64/aarch64.c (aarch64_frame_pointer_required): Check
LR_REGNUM.
(aarch64_can_eliminate): Don't check elimination source when
frame_pointer_requred is false.
Index: ../../gcc/gcc/config/aarch64/aarch64.c
===================================================================
--- ../../gcc/gcc/config/aarch64/aarch64.c (revision 205590)
+++ ../../gcc/gcc/config/aarch64/aarch64.c (working copy)
@@ -1703,7 +1703,7 @@ aarch64_frame_pointer_required (void)
if (flag_omit_frame_pointer && !faked_omit_frame_pointer)
return false;
else if (flag_omit_leaf_frame_pointer)
- return !crtl->is_leaf;
+ return !crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM);
return true;
}
@@ -4126,7 +4126,7 @@ aarch64_can_eliminate (const int from, c
of faked_omit_frame_pointer here (which is true when we always
wish to keep non-leaf frame pointers but only wish to keep leaf frame
pointers when LR is clobbered). */
- if (from == FRAME_POINTER_REGNUM && to == STACK_POINTER_REGNUM
+ if (to == STACK_POINTER_REGNUM
&& df_regs_ever_live_p (LR_REGNUM)
&& faked_omit_frame_pointer)
return false;