Re: input address reload issue

[Jeff Law <law@redhat.com>]

On 01/09/2017 07:02 AM, Aurelien Buhrig wrote:
> On 06/01/2017 17:06, Jeff Law wrote:
>> On 01/06/2017 03:20 AM, Aurelien Buhrig wrote:
>>>
>>>>> So the insn:
>>>>> (set (reg:QI 0 r0) (mem:QI (plus:SI (reg:SI 2 r2)(const_int 1))
>>>>>
>>>>> is transformed into:
>>>>> (set (reg:SI 8 a0) (reg:SI 2 r2))
>>>>> (set (reg:SI 8 a0) (const_int 1))
>>>>> (set (reg:SI 8 a0) (plus:SI (reg:SI 8 a0) (reg:SI 8 a0)))
>>>>> (set (reg:QI 0 r0) (mem:QI (reg:SI 8 a0))
>>>>>
>>>>> This "basic" transformation requires two reload regs, but only one is
>>>>> given/used/possible from the rl structure (in emit_reload_insns).
>>>>>
>>>>> So where does the issue comes from? The need for 2 reload regs, the
>>>>> transformation which is too "basic" and could be optimized to use only
>>>>> one reload reg, or any wrong/missing reload target hook?
>>>> Sounds like you need secondary or intermediate reloads.
>>> Do you suggest the secondary reload must implement a scratch reg & md
>>> pattern to implement this reload?
>> Perhaps.  I don't know enough about your architecture to be 100% sure
>> about how all the pieces interact with each other -- reload, and
>> secondary reloads in particular are a complex area.  I'm largely going
>> on your comment that you need 2 reload registers.
>>
>> Presumably you don't have an instruction for
>> (set (reg) (plus (reg) (const_int)))
>>
>> Thus you need two scratch reload regs IIUC.  One to hold r2 another to
>> hold (const_int 1).  So you'd want to generate
>>
>> (set (areg1) (reg r2))
>> (set (areg2) (const_int 1))
>> (set (areg1) (plus (areg1) (areg2)
>> (set (r0) (mem (areg1))
>>
>> Or something along those lines.  If you're going to stick with reload,
>> you'll likely want to dig into find_reloads_address and its children
>> to see what reloads it generates and why (debug_reload can be helpful
>> here).
>
> Thank you very much Jeff for your help. The best mapping for my target
> would be:
> (set areg1 r2) (set r0 (mem:QI (areg1 + 1)), and only 1 scratch would be
> needed.
So if you've got reg+d addressing modes, then I'd start by looking at 
your GO_IF_LEGITIMATE_ADDRESS, REG_OK_FOR_*_P, etc.  Reload ought to be 
able to reload r2 into an address register without defining secondary 
reloads.  From what you've described, it's comparable to any target that 
has split address/data registers.
>
> So I try to add LEGITIMIZE_RELOAD_ADDRESS. I can see the (mem:QI (r2+1))
> with hwreg, and so try to push_reload r2 with the "BASE_REG_CLASS".
> But it has no effect. Indeed, in push_secondary_reload, the
> secondary_reload is called and if NO_REG and CODE_FOR_nothing is
> returned, "no reload reg is needed" ... But r2 is a hwreg which does not
> belong to BASE_REG_CLASS, can be directly copied to BASE_REG_CLASS and
> it does need a reload reg.
Never use LEGITIMIZE_RELOAD_ADDRESS to fix a correctness issue.  It is 
strictly a way to improve the generated code.  It's primary purpose is 
to allow improvement of reg+d addresses when the displacement is out of 
range.

Jeff