Hi Wilco,

> On 18 Oct 2018, at 19:08, Wilco Dijkstra <wilco.dijks...@arm.com> wrote:

>> I wondered if we could set it to R11 unconditionally and picked
>> the way ensuring no change for !vxworks ports, especially since I
>> don't have means to test more than what I described above.
> 
> Yes it should always be the same register, there is no gain in switching
> it dynamically. I'd suggest to use X9 since X8 is the last register used for
> arguments (STATIC_CHAIN_REGNUM is passed when calling a nested
> function) and some of the higher registers may be used as temporaries in
> prolog/epilog.

Thanks for your feedback!  I ported the patches
to gcc-8 and was able to get a functional toolchain
for aarch64-wrs-vxworks7 and aarch64-elf, passing
full Acats for a couple of runtime variants on VxWorks
(compiled for RTP or kernel mode) as well as a small
internal testsuite we have, dedicated to cross configurations.

All the patches apply directly on mainline.

As for the original patch, I also sanity checked that
"make all-gcc" passes (self tests included) there for
--target=aarch64-elf --enable-languages=c


There are three changes to the common aarch64 port files.

It turns out that X9 doesn't work for STATIC_CHECK_REGNUM
because this conflicts with the registers used for -fstack-check:

  /* The pair of scratch registers used for stack probing.  */
  #define PROBE_STACK_FIRST_REG  9
  #define PROBE_STACK_SECOND_REG 10

I didn't find that immediately (read, I first experienced a few
badly crashing test runs) because I searched for R9_REGNUM to check
for other uses, so the first patchlet attached simply adjusts
the two #defines above to use R9/R10_REGNUM.


2018-10-23  Olivier Hainque  <hain...@adacore.com>

        * config/aarch64/aarch64.c (PROBE_STACK_FIRST_REG) : Redefine as
        R9_REGNUM instead of 9.
        (PROBE_STACK_SECOND_REG): Redefine as R10_REGNUM instead of 10.


The second patch is the one which I proposed a few days ago
to allow a subtarget (in my case, the VxWorks port) to state that
R18 is to be considered fixed. Two changes compared to the original
patch: a comment attached to the default definition of FIXED_R18,
and the unconditional use of R11_REGNUM as an alternate STATIC_CHAIN_REGNUM.

I suppose the latter could require extra testing than what I was
able to put in (since this is also changing for !vxworks configurations),
which Sam very kindly did on the first instance.

I didn't touch CALL_SAVED_REGISTERS since this is 1 for r18 already.
I also didn't see a strong reason to move to a more dynamic scheme,
through conditional_register_usage.

2018-03-18  Olivier Hainque  <hain...@adacore.com>

        * config/aarch64/aarch64.h (FIXED_R18): New internal
        configuration macro, defaulted to 0.
        (FIXED_REGISTERS): Use it.
        (STATIC_CHAIN_REGNUM): Use r11 instead of r18.


The third patch proposes the introduction of support for a
conditional SUBTARGET_OVERRIDE_OPTIONS macro, as many other
architectures have, and which is needed by all VxWorks ports.

In the current state, this one could possibly impact only
VxWorks, as no other config file would define the macro.

I'm not 100% clear on the possible existence of rules regarding
the placement of this within the override_options functions. We used
something similar to what other ports do, and it worked just fine
for VxWorks.

2018-10-23  Olivier Hainque  <hain...@adacore.com>

        * config/aarch64/aarch64.c (aarch64_override_options): Once
        arch, cpu and tune were validated, insert SUBTARGET_OVERRIDE_OPTIONS
        if defined.

I'm happy to adjust any of all this if needed of course.

Thanks in advance for your feedback!

With Kind Regards,

Olivier


From dd32f3611e4cb10f0b48d58d84c96951befbd99f Mon Sep 17 00:00:00 2001
From: Olivier Hainque <hain...@adacore.com>
Date: Sun, 21 Oct 2018 10:32:13 +0200
Subject: [PATCH 1/6] Use R9/R10_REGNUM to designate stack checking registers

---
 gcc/config/aarch64/aarch64.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 6810db7..f03e803 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3793,8 +3793,8 @@ aarch64_libgcc_cmp_return_mode (void)
 #endif
 
 /* The pair of scratch registers used for stack probing.  */
-#define PROBE_STACK_FIRST_REG  9
-#define PROBE_STACK_SECOND_REG 10
+#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.  */
-- 
1.7.10.4

From 399f0f4773bc3771f2a03c1e750f3b09d6b4824f Mon Sep 17 00:00:00 2001
From: Olivier Hainque <hain...@adacore.com>
Date: Fri, 19 Oct 2018 12:54:26 +0200
Subject: [PATCH 2/6] Introduce FIXED_R18 in aarch64.h

---
 gcc/config/aarch64/aarch64.h |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 976f9af..4069e29 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -267,11 +267,17 @@ extern unsigned aarch64_architecture_version;
 
 /* Standard register usage.  */
 
+/* Whether R18, the "platform register", is used as such by the target
+   environment, and may thus not be used as a scratch register.  This may
+   be redefined to 1 by os specific configurations.  */
+#define FIXED_R18 0
+
 /* 31 64-bit general purpose registers R0-R30:
    R30         LR (link register)
    R29         FP (frame pointer)
    R19-R28     Callee-saved registers
-   R18         The platform register; use as temporary register.
+   R18         The platform register; available as temporary register
+                unless FIXED_R18 is set.
    R17         IP1 The second intra-procedure-call temporary register
                (can be used by call veneers and PLT code); otherwise use
                as a temporary register
@@ -316,7 +322,7 @@ extern unsigned aarch64_architecture_version;
   {                                                    \
     0, 0, 0, 0,   0, 0, 0, 0,  /* R0 - R7 */           \
     0, 0, 0, 0,   0, 0, 0, 0,  /* R8 - R15 */          \
-    0, 0, 0, 0,   0, 0, 0, 0,  /* R16 - R23 */         \
+    0, 0, FIXED_R18, 0, 0, 0, 0, 0,    /* R16 - R23 */         \
     0, 0, 0, 0,   0, 1, 0, 1,  /* R24 - R30, SP */     \
     0, 0, 0, 0,   0, 0, 0, 0,   /* V0 - V7 */           \
     0, 0, 0, 0,   0, 0, 0, 0,   /* V8 - V15 */         \
@@ -403,7 +409,7 @@ extern unsigned aarch64_architecture_version;
    uses alloca.  */
 #define EXIT_IGNORE_STACK      (cfun->calls_alloca)
 
-#define STATIC_CHAIN_REGNUM            R18_REGNUM
+#define STATIC_CHAIN_REGNUM            R11_REGNUM
 #define HARD_FRAME_POINTER_REGNUM      R29_REGNUM
 #define FRAME_POINTER_REGNUM           SFP_REGNUM
 #define STACK_POINTER_REGNUM           SP_REGNUM
-- 
1.7.10.4

From 67ffe5c97d19e3a4aa04e3f9de92b77346988870 Mon Sep 17 00:00:00 2001
From: Olivier Hainque <hain...@adacore.com>
Date: Fri, 19 Oct 2018 21:19:52 +0200
Subject: [PATCH 3/6] Support for SUBTARGET_OVERRIDE_OPTIONS in aarch64.c

---
 gcc/config/aarch64/aarch64.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index f03e803..b4fedbd 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10893,6 +10893,10 @@ aarch64_override_options (void)
   if (aarch64_tune_string)
     valid_tune = aarch64_validate_mtune (aarch64_tune_string, &selected_tune);
 
+#ifdef SUBTARGET_OVERRIDE_OPTIONS
+  SUBTARGET_OVERRIDE_OPTIONS;
+#endif
+
   /* If the user did not specify a processor, choose the default
      one for them.  This will be the CPU set during configuration using
      --with-cpu, otherwise it is "generic".  */
-- 
1.7.10.4

Reply via email to