On Tue, Jan 6, 2015 at 7:36 AM, Vladimir Makarov <vmaka...@redhat.com> wrote:
>
> On 2015-01-05 12:31 PM, Jeff Law wrote:
>>
>> On 01/05/15 00:44, Kito Cheng wrote:
>>>
>>> Hi Vladimir:
>>> This patch has a discusses with you in May 2014, this patch is about
>>> the caller-save register store and restore instruction generation, the
>>> current LRA implementation will miss caller-save store/restore
>>> instruction if need one more instruction.
>>>
>>> You said you will investigate for this on IRC, so I don't send the
>>> patch last year, however ARM guys seem got this problem too, so I
>>> think it's time to send this patch :)
>>>
>>> ChangeLog
>>>
>>> 2015-01-05 Kito Cheng <k...@0xlab.org>
>>>
>>> * lra-constraints.c (split_reg): Fix caller-save store/restore
>>> instruction generation.
>>
>> Please reference PR64348 in the ChangeLog entry.
>>
>> Please include a testcase if there isn't one in the regression suite
>> already.
>>
>> Please indicate what platform this patch was bootstrapped and regression
>> tested on.
>>
>> The dumping code immediately after the assert you removed has code like
>> this in both cases:
>>
>>
>>
>> fprintf (lra_dump_file,
>> " Rejecting split %d->%d "
>> "resulting in > 2 %s restore insns:\n",
>> original_regno, REGNO (new_reg), call_save_p ? "call" :
>> "");
>>
>> Testing call_save_p here won't make any sense after your patch.
>>
>> I'll let Vlad chime in on the correctness of allowing multi register
>> saves/restores in this code.
>>
> The solution itself is ok. Prohibiting generation of more one insn was
> intended for inheritance only as inheritance transformation can be undone
> when the inheritance pseudo does not get a hard register. Undoing
> multi-register splitting is difficult and also such splitting is doubtedly
> profitable.
>
> Splitting for save/restore is never undone. So it is ok for this case to
> generate multi-register saves/restores.
>
> Kito, Jeff wrote reasonable changes for the patch. Please, do them and you
> can commit the patch.
Hi Vlad,
As for this specific case in PR64348, dump IR crossing function call
is as below:
430: [sfp:SI-0x30]=r989:TI#0
432: [r1706:SI+0x4]=r989:TI#4
434: [r1706:SI+0x8]=r989:TI#8
436: [r1706:SI+0xc]=r989:TI#12
441: r0:DI=call [`__aeabi_idivmod'] argc:0
REG_UNUSED r0:SI
REG_CALL_DECL `__aeabi_idivmod'
REG_EH_REGION 0xffffffff80000000
437: r1007:SI=sign_extend(r989:TI#0)
REG_DEAD r989:TI
Save/restore are introduced because of use of r989:#0 in insn 437.
Register r989 is TImode register and assigned to r2/r3/r4/r5. I can
think about two possible improvements to LRA splitter. 1) According
to ARM EABI, only r2/r3 need to be saved/restored; 2) In this case, we
only need to save/restore r989:TI#0, which is r2. In fact, it has
already been saved to stack in insn 430, what we need is to restore
r989:TI#0 after function call.
What do you think about these?
Thanks,
bin
>
> Thanks.
>
