This is the mail archive of the gcc-patches@gcc.gnu.org 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


Kyrill Tkachov <kyrylo.tkachov@arm.com> writes:
> 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.

Sorry for the breakage.  I've reverted the patch for now and will send a
fixed version when I have time.

Thanks,
Richard


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