diff mbox series

[aarch64] Remove obsolete comment about X30

Message ID 20180618134304.15979-1-siddhesh@sourceware.org
State New
Headers show
Series [aarch64] Remove obsolete comment about X30 | expand

Commit Message

Siddhesh Poyarekar June 18, 2018, 1:43 p.m. UTC
r217431 changed X30 as caller-saved in CALL_USE_REGISTERS because of
which this comment about X30 not being marked as call-clobbered is no
longer accurate.

Siddhesh

	* config/aarch64/aarch64.h: Remove obsolete comment.
---
 gcc/config/aarch64/aarch64.h | 9 ---------
 1 file changed, 9 deletions(-)

Comments

James Greenhalgh June 19, 2018, 3:41 p.m. UTC | #1
On Mon, Jun 18, 2018 at 08:43:04AM -0500, Siddhesh Poyarekar wrote:
> r217431 changed X30 as caller-saved in CALL_USE_REGISTERS because of
> which this comment about X30 not being marked as call-clobbered is no
> longer accurate.

Is the second paragraph is still relevant to how we define EPILOGUE_USES?

Possibly I'd rewrite the comment to explain the behaviour around calls and
how they interact with x30.

It seems like the right thing to do, but I can't shake the feeling we'd be
losing useful information here.

James

> 	* config/aarch64/aarch64.h: Remove obsolete comment.
> ---
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 976f9afae54..df9fb31aa64 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -303,15 +303,6 @@ extern unsigned aarch64_architecture_version;
>     register.  GCC internally uses the poly_int variable aarch64_sve_vg
>     instead.  */
>  
> -/* 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.  */
> -
>  #define FIXED_REGISTERS					\
>    {							\
>      0, 0, 0, 0,   0, 0, 0, 0,	/* R0 - R7 */		\
> -- 
> 2.14.4
>
Siddhesh Poyarekar June 20, 2018, 9:41 a.m. UTC | #2
On 06/19/2018 09:11 PM, James Greenhalgh wrote:
> On Mon, Jun 18, 2018 at 08:43:04AM -0500, Siddhesh Poyarekar wrote:
>> r217431 changed X30 as caller-saved in CALL_USE_REGISTERS because of
>> which this comment about X30 not being marked as call-clobbered is no
>> longer accurate.
> 
> Is the second paragraph is still relevant to how we define EPILOGUE_USES?

It is, but it is essentially a repetition of the comment directly above 
EPILOGUE_USES.

> Possibly I'd rewrite the comment to explain the behaviour around calls and
> how they interact with x30.

How about this then:

Siddhesh
From 51d1dd0ed45c3dde38192677f574f2259b869aca Mon Sep 17 00:00:00 2001
From: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date: Wed, 20 Jun 2018 15:06:44 +0530
Subject: [PATCH] [aarch64] Fix obsolete comment about X30

r217431 changed X30 as caller-saved in CALL_USE_REGISTERS because of
which this comment about X30 not being marked as call-clobbered is no
longer accurate.  Fixed to describe the current state more accurately.

Siddhesh

	* config/aarch64/aarch64.h (CALL_USE_REGISTERS): Fix obsolete
	comment.
	(EPILOGUE_USES): Likewise.
---
 gcc/config/aarch64/aarch64.h | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 976f9afae54..f284e74bfb8 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -303,15 +303,6 @@ extern unsigned aarch64_architecture_version;
    register.  GCC internally uses the poly_int variable aarch64_sve_vg
    instead.  */
 
-/* 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.  */
-
 #define FIXED_REGISTERS					\
   {							\
     0, 0, 0, 0,   0, 0, 0, 0,	/* R0 - R7 */		\
@@ -327,6 +318,13 @@ extern unsigned aarch64_architecture_version;
     0, 0, 0, 0,   0, 0, 0, 0,   /* P8 - P15 */          \
   }
 
+/* X30 is marked as caller-saved which is in line with regular function call
+   behavior since the call instructions clobber it; AARCH64_EXPAND_CALL does
+   that for regular function calls and avoids it for sibcalls.  X30 is
+   considered live for sibcalls; EPILOGUE_USES helps achieve that by returning
+   true but not until function epilogues have been generated.  This ensures
+   that X30 is available for use in leaf functions if needed.  */
+
 #define CALL_USED_REGISTERS				\
   {							\
     1, 1, 1, 1,   1, 1, 1, 1,	/* R0 - R7 */		\
@@ -391,9 +389,10 @@ extern unsigned aarch64_architecture_version;
     V_ALIASES(28), V_ALIASES(29), V_ALIASES(30), V_ALIASES(31)  \
   }
 
-/* Say that the epilogue uses the return address register.  Note that
-   in the case of sibcalls, the values "used by the epilogue" are
-   considered live at the start of the called function.  */
+/* Say that the return address register is used by the epilogue, but only after
+   epilogue generation is complete.  Note that in the case of sibcalls, the
+   values "used by the epilogue" are considered live at the start of the called
+   function.  */
 
 #define EPILOGUE_USES(REGNO) \
   (epilogue_completed && (REGNO) == LR_REGNUM)
James Greenhalgh June 26, 2018, 9:51 p.m. UTC | #3
On Wed, Jun 20, 2018 at 04:41:18AM -0500, Siddhesh Poyarekar wrote:
> On 06/19/2018 09:11 PM, James Greenhalgh wrote:
> > On Mon, Jun 18, 2018 at 08:43:04AM -0500, Siddhesh Poyarekar wrote:
> >> r217431 changed X30 as caller-saved in CALL_USE_REGISTERS because of
> >> which this comment about X30 not being marked as call-clobbered is no
> >> longer accurate.
> > 
> > Is the second paragraph is still relevant to how we define EPILOGUE_USES?
> 
> It is, but it is essentially a repetition of the comment directly above 
> EPILOGUE_USES.
> 
> > Possibly I'd rewrite the comment to explain the behaviour around calls and
> > how they interact with x30.
> 
> How about this then:

OK,

Thanks,
James
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 976f9afae54..df9fb31aa64 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -303,15 +303,6 @@  extern unsigned aarch64_architecture_version;
    register.  GCC internally uses the poly_int variable aarch64_sve_vg
    instead.  */
 
-/* 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.  */
-
 #define FIXED_REGISTERS					\
   {							\
     0, 0, 0, 0,   0, 0, 0, 0,	/* R0 - R7 */		\