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

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

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