Re: [ARM/FDPIC v5 04/21] [ARM] FDPIC: Add support for FDPIC for arm architecture

[Richard Sandiford <richard.sandiford@arm.com>]

Sorry for the slow reply.

Christophe Lyon <christophe.lyon@st.com> writes:
> On 16/07/2019 13:58, Richard Sandiford wrote:
>> Christophe Lyon <christophe.lyon@st.com> writes:
>>> +(define_insn "*restore_pic_register_after_call"
>>> +  [(parallel [(unspec [(match_operand:SI 0 "s_register_operand" "=r,r")
>>> +		       (match_operand:SI 1 "nonimmediate_operand" "r,m")]
>>> +	       UNSPEC_PIC_RESTORE)
>>> +	      (use (match_dup 1))
>>> +	      (clobber (match_dup 0))])
>>> +  ]
>>> +  ""
>>> +  "@
>>> +  mov\t%0, %1
>>> +  ldr\t%0, %1"
>>> +)
>>> +
>>>   (define_expand "call_internal"
>>>     [(parallel [(call (match_operand 0 "memory_operand" "")
>>>   	            (match_operand 1 "general_operand" ""))
>> 
>> Since operand 0 is significant after the instruction, I think this
>> should be:
>> 
>> (define_insn "*restore_pic_register_after_call"
>>    [(set (match_operand:SI 0 "s_register_operand" "+r,r")
>> 	(unspec:SI [(match_dup 0)
>> 		    (match_operand:SI 1 "nonimmediate_operand" "r,m")]
>> 		   UNSPEC_PIC_RESTORE))]
>>    ...
>> 
>> The (use (match_dup 1)) looks redundant, since the unspec itself
>> uses operand 1.
>> 
> When I try that, I have cases where the restore instruction is discarded, when the call happens just before function return. Since r9 is caller-saved, it should be restored but after dse2 the dumps say:
> (insn (set (reg:SI 9 r9)
>     (unspec:SI [
>         (reg:SI 9 r9)
>         (reg:SI 4 r4 [121])
>       ] UNSPEC_PIC_RESTORE))
> (expr_list:REG_UNUSED (reg:SI 9 r9) (nil))))
>
> and this is later removed by cprop_hardreg (which says the exit block uses r4, sp, and lr: should I make it use r9?)

But if it's caller-saved (i.e. call-clobbered), function A shouldn't
need to restore r9 after a call unless A needs the value of r9 for
something.  I.e. A shouldn't need to restore r9 for A's own caller,
because the caller should be doing that iself.

So if r9 is caller-saved and not referenced between the call and
function exit, deleting the restore sounds like the right thing to do.

Richard