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

[Richard Sandiford <rdsandiford at googlemail dot com>]

Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
>> Hmm, marking them fixed was supposed to be a temporary reload-only thing,
>> until the move to LRA.  It should never be worse to spill to these GPRs
>> over spilling to the stack, if the value isn't live across a call.
>
> I would say this also affects IRA/LRA integration. I found that it is more
> profitable to hide registers (MIPS16 only) in IRA to encourage spilling to
> memory. Otherwise $8-$15 would be treated like any other registers and LRA
> would inserts reloads to move in/out values of these registers. My assumption is
> that if we could hide some of the registers in IRA but enable them in LRA
> then all registers in SPILL_REGS would be available keeping reasonable code
> size. Another way would be to increase the cost of moving values between 
> M16_REGS and GR_REGS but it was already mentioned, and is true that there 
> should be no difference of costs and it feels like a hack to make things work.

OK.

This definitely sounds like it ought to be made to work, with some mixture
of target and generic changes.  But if it doesn't work out of the box
then let's leave that for future work.

>> Did you see the failures even after your mips_regno_mode_ok_for_base_p
>> change?  LRA should know how to reload a "W" address.
>
> Yes but I realize there is more. It fails because $sp is now included
> in BASE_REG_CLASS and "W" is based on it. However, I suppose that 
> it would be too eager to say it is wrong and likely there is something missing 
> in LRA if we want to keep all alternatives. Currently there is no check
> if a reloaded operand has a valid address, use of $sp in lbu/lhu cases.
> Even if we added extra checks we are less likely to benefit as we need
> to reload the base into register.

Not sure what you mean, sorry.  "W" exists specifically to exclude
$sp-based and $pc-based addresses.  LRA AFAIK should already be able
to reload addresses that are valid in the TARGET_LEGITIMATE_ADDRESS_P
sense but which do not match the constraints for a particular insn.

Can you remember one of the tests that fails?

>> Even that might be too loose, since invalid scales will need to be reloaded
>> as a multiplication or shift, and there's no guarantee that the target
>> can do that without clobbering the flags.  So maybe we should do something
>> like the patch below.
>>
>> Alternatively we could stick to the decompose_mem_address-based check
>> above and teach LRA to keep invalid addresses for 'X'.  The problem then
>> is that we might ICE while printing the operand.
>
> Tightening validity for 'X' appears to be reasonable. Will you commit
> this patch?

OK, just submitted separately.

Thanks,
Richard