This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: Vladimir Makarov <vmakarov at redhat dot com>, Robert Suchanek <Robert dot Suchanek at imgtec dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, rdsandiford at googlemail dot com
- Date: Tue, 06 May 2014 15:06:23 +0100
- Subject: Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
- Authentication-results: sourceware.org; auth=none
- References: <B5E67142681B53468FAF6B7C313565623D3DD70C at KLMAIL01 dot kl dot imgtec dot org> <87d2h51dm6 dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E3FED at KLMAIL01 dot kl dot imgtec dot org> <87d2gqfb7t dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E4420 at KLMAIL01 dot kl dot imgtec dot org> <87ob02jodp dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E460A at KLMAIL01 dot kl dot imgtec dot org> <87fvl6hnw2 dot fsf at talisman dot default> <5357D588 dot 6000202 at redhat dot com> <87tx967jnq dot fsf at talisman dot default>
On 03/05/14 20:21, Richard Sandiford wrote:
Vladimir Makarov <vmakarov@redhat.com> writes:
Not sure how the constraint would/should exclude $sp-based address in
LRA. In this particular case, a spilled pseudo is changed to memory
giving the following RTL form:
(insn 30 29 31 4 (set (reg:SI 4 $4)
(and:SI (mem/c:SI (plus:SI (reg/f:SI 78 $frame)
(const_int 16 [0x10])) [7 %sfp+16 S4 A32])
(const_int 65535 [0xffff]))) shell.i:17 161 {*andsi3_mips16}
(expr_list:REG_DEAD (reg:SI 194 [ D.1469 ])
(nil)))
The operand 1 during alternative selection is not marked as a bad
operand as it is a memory operand. $frame appears to be fine as it
could be eliminated later to hard register. No reloads are inserted
for the instructions concerned. Unless, $frame should be temporarily
eliminated and then a reload would be inserted?
Yeah, I think the lack of elimination is the problem. process_address
eliminates $frame temporarily before checking whether the address
is valid, but the places that check EXTRA_CONSTRAINT_STR pass the
original uneliminated address. So the legitimate_address_p hook sees
the $sp-based address but the "W" constraint only sees the $frame-based
address (which might or might not be valid, depending on whether $frame
is eliminated to the stack or hard frame pointer). I think the constraints
should see the eliminated address too.
This patch seems to fix it for me. Tested on x86_64-linux-gnu.
Vlad, is this OK for trunk?
BTW, we might want to define something like:
#define MODE_BASE_REG_CLASS(MODE) \
(TARGET_MIPS16 \
? ((MODE) == SImode || (MODE) == DImode ? M16_SP_REGS : M16_REGS) \
: GR_REGS)
instead of BASE_REG_CLASS. It might lead to slightly better code
(or not -- if it doesn't then don't bother :-)).
If this patch is OK then I think the only thing blocking the switch
to LRA is the asm-subreg-1.c failure. I think it'd be fine to XFAIL
that test on MIPS for now, until there's a consensus about what "X" means
for asms.
gcc/
* lra-constraints.c (valid_address_p): Move earlier in file.
Add a constraint argument to the address_info version.
(satisfies_memory_constraint_p): New function.
(satisfies_address_constraint_p): Likewise.
(process_alt_operands, curr_insn_transform): Use them.
(process_address): Pass the constraint to valid_address_p when
checking address operands.
Yes, it looks ok for me, Richard. Thanks on working on this.
I am on vacation till May 4th. If the patch results in problems on other
targets, I hope you revert it. But to be honest, I believe it is very
safe and don't expect any problems at all.
Thanks Vlad, belatedly committed on that basis. Like you say I'll revert
it at the first sign of trouble (although it ended up being closer to
your return than originally intended. :-))
Hi all,
This caused some testsuite failures on arm:
FAIL: gcc.target/arm/vfp-ldmdbd.c scan-assembler fldmdbd
FAIL: gcc.target/arm/vfp-ldmdbs.c scan-assembler fldmdbs
FAIL: gcc.target/arm/vfp-ldmiad.c scan-assembler fldmiad
FAIL: gcc.target/arm/vfp-ldmias.c scan-assembler fldmias
From the vfp-ldmdbd.c test this patch changed the codegen from:
fldmdbd r5!, {d7}
into
sub r5, r5, #8
fldd d7, [r5]
Which broke the test.
Kyrill