public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] follow up on the aarch64 r18 story
@ 2019-11-07 16:16 Olivier Hainque
  2019-11-15 15:47 ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 6+ messages in thread
From: Olivier Hainque @ 2019-11-07 16:16 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 2801 bytes --]

Hello,

About a year ago we discussed various possibilities regarding
an issue with the aarch64 selection of r18 as the static chain,
problematic in environments where the OS uses r18 for "private"
reasons. VxWorks uses this permission to use r18 as a pointer to
the current TCB, and Windows does something similar I think.

I first proposed a change allowing target ports to state that
r18 should be considered "fixed" and switching the static chain
to r11 in this case. I had picked r11 as the next after r9/r10
already used for stack checking,

After a few exchanges, we agreed that we should rather use r9
as the alternate static chain and remap the temporary registers
used for stack-checking to r10/r11.

  https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01592.html

We were also thinking that we should be able to change the
static chain register unconditionally.

Then during the last GNU cauldron, we realized that libffi
currently relies on the knowledge of which register is used for
the static chain, for the support of Go closures.

Unconditionally changing r18 to something else would break that
support, most notably on Linux. Adjusting libffi is possible but
raises a clear compatibility issue.

The attached patch is a renewed proposal to let target OS
ports state their private use of r18, switching the static
chain to r9 and declaring r18 "fixed" in that case. 

In addition to adjusting PROBE_STACK reg values to r10/r11,
the patch also moves the corresponding definitions together
with the other special register number definitions, so we have
a centralized view of the numbering choices.

This solves the immediate issue for VxWorks (which I'd like
to contribute) and seems like an interesting thing to have
in any case, regardless of whether or not we want to more
generally change the static chain someday.

With this change, we have good build and testing results
on both aarch64-elf and aarch64-vxworks7 for a gcc-9 based
compiler, where only vxworks states TARGET_OS_USES_R18.

I have also verified that I would build a native aarch64-linux
mainline compiler with the change and that it still picks r18 as
the static chain.

Is this variant ok to commit ?

Thanks in advance,

Best Regards,

Olivier

2019-11-07  Olivier Hainque  <hainque@adacore.com>

	* config/aarch64/aarch64.h: Define PROBE_STACK_FIRST_REG
	and PROBE_STACK_SECOND_REG here, to r10/r11 respectively.
	(TARGET_OS_USES_R18): New macro, default value 0 that target
	OS configuration files may redefine.
	(STATIC_CHAIN_REGNUM): r9 if TARGET_OS_USES_R18, r18 otherwise.
	* config/aarch64/aarch64.c (PROBE_STACK_FIRST_REG,
	PROBE_STACK_SECOND_REG): Remove definitions.
	(aarch64_conditional_register_usage): Preserve r18 if the target
	OS uses it, and check that the static chain selection wouldn't
	conflict.


[-- Attachment #2: aarch64-r18.diff --]
[-- Type: application/octet-stream, Size: 2201 bytes --]

diff --git gcc/config/aarch64/aarch64.c gcc/config/aarch64/aarch64.c
index 1dfff33..36d2814 100644
--- gcc/config/aarch64/aarch64.c
+++ gcc/config/aarch64/aarch64.c
@@ -5435,10 +5435,6 @@ aarch64_libgcc_cmp_return_mode (void)
 #error Cannot use simple address calculation for stack probing
 #endif
 
-/* The pair of scratch registers used for stack probing.  */
-#define PROBE_STACK_FIRST_REG  R9_REGNUM
-#define PROBE_STACK_SECOND_REG R10_REGNUM
-
 /* Emit code to probe a range of stack addresses from FIRST to FIRST+POLY_SIZE,
    inclusive.  These are offsets from the current stack pointer.  */
 
@@ -15527,6 +15523,14 @@ aarch64_conditional_register_usage (void)
       fixed_regs[SPECULATION_SCRATCH_REGNUM] = 1;
       call_used_regs[SPECULATION_SCRATCH_REGNUM] = 1;
     }
+
+  /* If the target OS has a private use of R18, preserve it.  */
+  if (TARGET_OS_USES_R18)
+    {
+      fixed_regs[R18_REGNUM] = 1;
+      call_used_regs[R18_REGNUM] = 1;
+      gcc_assert(STATIC_CHAIN_REGNUM != R18_REGNUM);
+    }
 }
 
 /* Walk down the type tree of TYPE counting consecutive base elements.
diff --git gcc/config/aarch64/aarch64.h gcc/config/aarch64/aarch64.h
index 425a363..3fccd2d 100644
--- gcc/config/aarch64/aarch64.h
+++ gcc/config/aarch64/aarch64.h
@@ -472,13 +472,22 @@ extern unsigned aarch64_architecture_version;
    uses alloca.  */
 #define EXIT_IGNORE_STACK	(cfun->calls_alloca)
 
-#define STATIC_CHAIN_REGNUM		R18_REGNUM
+/* Whether the target OS has a private use of R18, as allowed by the ABI.
+   OS specific configuration files may redefine this.  */
+#define TARGET_OS_USES_R18 0
+
+#define STATIC_CHAIN_REGNUM		(TARGET_OS_USES_R18 \
+					 ? R9_REGNUM : R18_REGNUM)
 #define HARD_FRAME_POINTER_REGNUM	R29_REGNUM
 #define FRAME_POINTER_REGNUM		SFP_REGNUM
 #define STACK_POINTER_REGNUM		SP_REGNUM
 #define ARG_POINTER_REGNUM		AP_REGNUM
 #define FIRST_PSEUDO_REGISTER		(FFRT_REGNUM + 1)
 
+/* The pair of scratch registers used for stack probing during prologue.  */
+#define PROBE_STACK_FIRST_REG		R10_REGNUM
+#define PROBE_STACK_SECOND_REG		R11_REGNUM
+
 /* The number of argument registers available for each class.  */
 #define NUM_ARG_REGS			8
 #define NUM_FP_ARG_REGS			8

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-01-09 10:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 16:16 [patch] follow up on the aarch64 r18 story Olivier Hainque
2019-11-15 15:47 ` Richard Earnshaw (lists)
2019-11-21 23:23   ` Olivier Hainque
2019-11-29 10:31     ` Olivier Hainque
2019-12-19 17:39       ` [patch] move and adjust PROBE_STACK_*_REG on aarch64 Olivier Hainque
2020-01-09 10:48         ` [ping] " Olivier Hainque

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).