public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>
To: Jiong Wang <jiong.wang@foss.arm.com>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: Marcus Shawcroft <Marcus.Shawcroft@arm.com>,
	James Greenhalgh <james.greenhalgh@arm.com>
Subject: Re: [AArch64] Update comments on the usage of X30 in FIXED_REGISTERS and CALL_USED_REGISTERS
Date: Mon, 02 Nov 2015 14:52:00 -0000	[thread overview]
Message-ID: <563778A3.1050301@foss.arm.com> (raw)
In-Reply-To: <56375DFB.7050504@foss.arm.com>

On 02/11/15 12:58, Jiong Wang wrote:
> 
> 
> On 02/11/15 12:01, Richard Earnshaw wrote:
>> On 16/10/15 15:36, Jiong Wang wrote:
>>> The patch https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02654.html
>>> from last year changed the definition of LR in CALL_USED_REGISTERS,
>>> but didn't update the comment above the #define to reflect the new
>>> usage.
>>>
>>> This patch bring the comment inline with the implementation.
>>>
>>> OK for trunk?
>>>
>>> Thanks.
>>>
>>> 2015-10-16  Jiong. Wang  <jiong.wang@arm.com>
>>>
>>> gcc/
>>>    * config/aarch64/aarch64.h: Update the comments on usage of X30.
>>>
>>>
>>> fix-comment.patch
>>>
>>>
>>> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
>>> index 5a8db76..1eaaca0 100644
>>> --- a/gcc/config/aarch64/aarch64.h
>>> +++ b/gcc/config/aarch64/aarch64.h
>>> @@ -210,14 +210,17 @@ extern unsigned aarch64_architecture_version;
>>>      significant bits.  Unlike AArch32 S1 is not packed into D0,
>>>      etc.  */
>>>   -/* Note that we don't mark X30 as a call-clobbered register.  The
>>> idea is
>>> -   that it's really the call instructions themselves which clobber X30.
>>> -   We don't care what the called function does with it afterwards.
>>> -
>>> -   This approach makes it easier to implement sibcalls.  Unlike normal
>>> -   calls, sibcalls don't clobber X30, so the register reaches the
>>> -   called function intact.  EPILOGUE_USES says that X30 is useful
>>> -   to the called function.  */
>>> +/* We don't mark X30 as a fixed register while we mark it as a
>>> caller-saved
>>> +   register.  The idea is we want X30 to be allocable as a caller-saved
>>> +   register when possible.
>>> +
>>> +   NOTE: although X30 is marked as caller-saved, it's callee-saved
>>> at the same
>>> +   time.  The caller-saved attribute makes sure if X30 is allocated
>>> as free
>>> +   register to hold any temporary value then the value is saved
>>> properly across
>>> +   function call.  While on AArch64, the call instruction writes the
>>> return
>>> +   address to LR.  If the called function is a non-leaf function, it
>>> is the
>>> +   responsibility of the callee to save and restore LR appropriately
>>> in it's
>>> +   prologue / epilogue.  */
>>>   
>> Sorry, but I find that just confusing.
>>
>> Wouldn't it be easier just to say:
>>
>>   X30 is clobbered by call instructions, so must be treated as a
>>   caller-saved register.
> 
> Richard, thanks for the review, but I am not convinced by your change.
> 
> "caller-saved" in gcc just means if the live range of the register is
> across function call then the caller will make sure it will be saved and
> restored properly. this is completely a calling convention concept and
> have not relationship with how call instruction works.
> 
> So, we mark X30 as caller-saved not because it will be clobbered by the
> call instructions but because we relax it as free register and want it
> to be saved by caller whenever it's allocated by register allocation and
> life range is across function call.
> 
> And from my understanding, if one register if clobbered by call
> instruction, it's not must be treated as a caller-saved register,
> instead it must be treated as a *callee-saved* register. Because the call
> instruction is actually assigning a new value to the register then jump
> to the callee in an atomic way thus there is no save/restore from the
> caller of this "new value", callee is full responsible for this. The
> "NOTE" part in the patch is trying to highlight this so following extra
> check in aarch64_layout_frame with be eaiser to understand for others.
> 
>   /* ... and any callee saved register that dataflow says is live. */
>   for (regno = R0_REGNUM; regno <= R30_REGNUM; regno++)
>     if (df_regs_ever_live_p (regno)
>         && (regno == R30_REGNUM   <--- X30, a caller-save, is
> callee-save as well.
>             || !call_used_regs[regno]))
>       cfun->machine->frame.reg_offset[regno] = SLOT_REQUIRED;
> 
> Regards,
> Jiong

Right, I think I now understand what you are trying to say, but I still
think the wording does not convey that.

We have two statements of fact
1) On entry to a function LR contains the return address (by the
architecture)
2) LR cannot retain values across a function call (it is a caller-saved
register by the PCS)

We then have an implementation perspective on how to use LR given these
constraints: we treat the register as a callee-saved register and put
explicit clobbers on all call instructions.

So how does the following sound?

/* Technically, LR should be treated as a caller-saved register (since
it is modified during a subroutine call to contain the return address).
 However, from the compiler's perspective, it is best to treat it as a
callee-saved register and then to put explicit clobber instructions on
each call instruction to ensure that live values are not retained in it
across call instructions.  This allows us to use the register as a
scratch register between function calls.  */

R.

  reply	other threads:[~2015-11-02 14:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16 14:44 Jiong Wang
2015-10-30 14:58 ` Jiong Wang
2015-11-02 12:02 ` Richard Earnshaw
2015-11-02 12:58   ` Jiong Wang
2015-11-02 14:52     ` Richard Earnshaw [this message]
2015-11-03 17:51       ` Jiong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=563778A3.1050301@foss.arm.com \
    --to=richard.earnshaw@foss.arm.com \
    --cc=Marcus.Shawcroft@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=james.greenhalgh@arm.com \
    --cc=jiong.wang@foss.arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).