This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend

On 03/05/14 20:21, Richard Sandiford wrote:
Vladimir Makarov <> 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 ])

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:

    (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.

	* 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: scan-assembler fldmdbd
FAIL: scan-assembler fldmdbs
FAIL: scan-assembler fldmiad
FAIL: scan-assembler fldmias

From the vfp-ldmdbd.c test this patch changed the codegen from:
    fldmdbd    r5!, {d7}

    sub    r5, r5, #8
    fldd    d7, [r5]

Which broke the test.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]