[gcc(refs/vendors/ARM/heads/morello)] Alternate method to set LSB on labels

[gcc(refs/vendors/ARM/heads/morello)] Alternate method to set LSB on labels

[Matthew Malcomson]

https://gcc.gnu.org/g:95c8fab042e81ebe6aeb31205019752d83a6d8e2

commit 95c8fab042e81ebe6aeb31205019752d83a6d8e2
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Thu Sep 16 11:33:53 2021 +0100

    Alternate method to set LSB on labels
    
    We needed to add one to these labels when loading directly as well as
    when storing in data.  The obvious way to set this LSB when loading
    directly was to put the increment as a hook in the expand pass.  This
    affects the approach when storing in data too.  Hence we remove the
    code we've added to include this LSB when storing as a constant and add
    the new method that accounts for storing as a constant and loading
    directly.
    
    Making this adjustment in the expand pass did mean we needed to add a
    new target hook.  We call it `TARGET_ADJUST_LABEL_EXPANSION` and call it
    when `expand_expr_real_1` is expanding a `LABEL_REF`.
    
    With this new approach, `aarch64_load_symref_appropriately` now can
    often be requested to load something of the form
      `(const (pointer_plus (label_ref) (const_int)))`.
    Unlike the other forms `aarch64_load_symref_appropriately` gets called
    with, this RTX is not one that can be shared between insns.
    (N.b. the same form with a `symbol_ref` can be shared according to
    `shared_const_p`).  `aarch64_load_symref_appropriately` was originally
    written to use the same immediate RTX expression it gets given in HIGH
    and LO_SUM expressions.
    
    When `aarch64_load_symref_appropriately` is given an RTX of the new form
    we need to ensure that we do not share that exact RTX between different
    insns.  This would cause an ICE when verifying that no RTL is invalidly
    shared.
    
    We fix that by using the result of `copy_rtx` in some of the insn
    emitting calls of aarch64_load_symref_appropriately.  `copy_rtx` already
    handles checking for whether we have a SYMBOL_REF or LABEL_REF and
    avoiding the actual copy for a SYMBOL_REF.
    
    Our test checks for both directly getting a label's value and for
    storing a label's value in the data section.  This will provide some
    test coverage until we can build and run programs on a model.  Since
    this change is essential to be able to jump to a label without changing
    execution mode building and running programs will provide good coverage
    of this feature from then on.

Diff:
---
 gcc/config/aarch64/aarch64.c                       | 54 ++++++++++++++--------
 gcc/doc/tm.texi                                    | 11 +++++
 gcc/doc/tm.texi.in                                 |  2 +
 gcc/expr.c                                         |  1 +
 gcc/target.def                                     | 14 ++++++
 .../morello/label-addressing-includes-lsb.c        | 32 +++++++++++++
 6 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e19a99ad206..864c14dee25 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3126,6 +3126,12 @@ static void
 aarch64_load_symref_appropriately (rtx dest, rtx imm,
 				   enum aarch64_symbol_type type)
 {
+  /* Can not share an RTX of the form ({pointer_,}plus (label_ref) (const_int)),
+     but can share RTX of the form ({pointer_,}plus (symbol_ref) (const_int)).
+     This is very rarely needed, but it is at least triggered when adding the
+     LSB to a label_ref for Morello.  Checking whether we actually need to do
+     the copy or not is done inside `copy_rtx`.  */
+  rtx imm2 = copy_rtx (imm);
   /* Assert that any time we're asked to load an indirection symbol there is no
      offset.  If there were an offset that would break one of the assumptions
      we have to believe that loading symbols is safe.  (I.e. if we're ever
@@ -3148,7 +3154,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
 	if (can_create_pseudo_p ())
 	  tmp_reg = gen_reg_rtx (mode);
 
-	emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm));
+	emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm2));
 	check_emit_insn (gen_add_losym (dest, tmp_reg, imm));
 	return;
       }
@@ -3245,7 +3251,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
 	if (can_create_pseudo_p ())
 	  tmp_reg = gen_reg_rtx (mode);
 
-	emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm));
+	emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm2));
 	if (mode == ptr_mode)
 	  {
 	    insn = gen_ldr_got_small (mode, dest, tmp_reg, imm);
@@ -3320,7 +3326,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
 
 	check_emit_insn (gen_rtx_SET (dest, gen_raw_pointer_plus (sa, tp, x0)));
 	if (REG_P (dest))
-	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm);
+	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2);
 	return;
       }
 
@@ -3355,7 +3361,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
 	check_emit_insn (gen_rtx_SET (dest,
 				gen_raw_pointer_plus (sa, tp, om_reg)));
 	if (REG_P (dest))
-	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm);
+	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2);
 	return;
       }
 
@@ -3393,7 +3399,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
 	  }
 
 	if (REG_P (dest))
-	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm);
+	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2);
 	return;
       }
 
@@ -3432,7 +3438,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
 	  }
 
 	if (REG_P (dest))
-	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm);
+	  set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2);
 	return;
       }
 
@@ -19888,18 +19894,15 @@ aarch64_asm_output_capability (rtx x, unsigned int size, int aligned_p)
       if (TARGET_CAPABILITY_FAKE)
 	return targetm.asm_out.integer(address_value, size, aligned_p);
 
-      if (SYMBOL_REF_P (cap_val)
-	  && SUBREG_P (address_value)
-	  && subreg_lowpart_p (address_value)
-	  && LABEL_REF_P (SUBREG_REG (address_value)))
-	{
-	  /* For a pointer to a label, we need to set the LSB to preserve
-	     PSTATE.C64 for purecap.  */
-	  if (TARGET_CAPABILITY_PURE)
-	    XEXP (x, 1) = plus_constant (POmode, address_value, 1);
-	}
-      else
-	gcc_unreachable ();
+      gcc_assert (SYMBOL_REF_P (cap_val)
+		  && SUBREG_P (address_value)
+		  && subreg_lowpart_p (address_value));
+      rtx sub = SUBREG_REG (address_value);
+      gcc_assert ((TARGET_CAPABILITY_PURE
+		   && GET_CODE (sub) == CONST
+		   && POINTER_PLUS_P (XEXP (sub, 0))
+		   && LABEL_REF_P (XEXP (XEXP (sub, 0), 0)))
+		  || (!TARGET_CAPABILITY_PURE && LABEL_REF_P (sub)));
     }
 
   /* Fake capability => size is the correct size and just want to emit the
@@ -24272,6 +24275,18 @@ aarch64_target_capability_mode ()
   return opt_scalar_addr_mode ();
 }
 
+/* Implement TARGET_ADJUST_LABEL_EXPANSION hook.  */
+rtx
+aarch64_adjust_label_expansion (rtx x)
+{
+  /* For a pointer to a label, we need to set the LSB to preserve PSTATE.C64
+     for purecap (jumps to a register will take PSTATE.C64 from the LSB set in
+     that register).  */
+  if (TARGET_CAPABILITY_PURE)
+    return plus_constant (GET_MODE (x), x, 1);
+  return x;
+}
+
 /* Target-specific selftests.  */
 
 #if CHECKING_P
@@ -24840,6 +24855,9 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_CAPABILITY_MODE
 #define TARGET_CAPABILITY_MODE aarch64_target_capability_mode
 
+#undef TARGET_ADJUST_LABEL_EXPANSION
+#define TARGET_ADJUST_LABEL_EXPANSION aarch64_adjust_label_expansion
+
 #undef TARGET_ASM_DECLARE_CONSTANT_NAME
 #define TARGET_ASM_DECLARE_CONSTANT_NAME aarch64_declare_constant_name
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 56fe193f363..982f51bcfc4 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -8207,6 +8207,17 @@ Even if the current target does not use the REPLACE_ADDRESS_VALUE RTL code
 this function will be given that code, so it must be handled by this hook.
 @end deftypefn
 
+@deftypefn {Target Hook} rtx TARGET_ADJUST_LABEL_EXPANSION (rtx @var{x})
+Adjusts the RTX that has come from expanding a label.  This RTX has come
+from an expansion of a label and is to be used as data (e.g. in a computed
+goto) rather than to be jumped to directly.  This hook should take @var{x}
+and return a new RTX adjusted with any machine-specific requirements.
+
+Most targets do not need to implement this hook.  The default returns
+@var{x} directly.  One target that does need to implement it is AArch64.
+AArch64 needs to set the LSB of labels depending on the processor state.
+@end deftypefn
+
 @deftypefn {Target Hook} void TARGET_ASM_DECL_END (void)
 Define this hook if the target assembler requires a special marker to
 terminate an initialized variable declaration.
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 6d037ef77e1..067be34c62c 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5226,6 +5226,8 @@ It must not be modified by command-line option processing.
 
 @hook TARGET_ASM_CAPABILITY
 
+@hook TARGET_ADJUST_LABEL_EXPANSION
+
 @hook TARGET_ASM_DECL_END
 
 @hook TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA
diff --git a/gcc/expr.c b/gcc/expr.c
index 3c682e0584c..a04b77f6309 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -10269,6 +10269,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode tmode,
 	    && function != 0)
 	  LABEL_REF_NONLOCAL_P (temp) = 1;
 
+	temp = targetm.adjust_label_expansion (temp);
 	temp = gen_rtx_MEM (FUNCTION_MODE, temp);
 	return temp;
       }
diff --git a/gcc/target.def b/gcc/target.def
index d20db1da93e..2571c5583c0 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -3254,6 +3254,20 @@ DEFHOOK
  opt_scalar_addr_mode.  This is the default.",
  opt_scalar_addr_mode, (), default_capability_mode)
 
+/* Machine specific adjustments when expanding a label.  */
+DEFHOOK
+(adjust_label_expansion,
+ "Adjusts the RTX that has come from expanding a label.  This RTX has come\n\
+from an expansion of a label and is to be used as data (e.g. in a computed\n\
+goto) rather than to be jumped to directly.  This hook should take @var{x}\n\
+and return a new RTX adjusted with any machine-specific requirements.\n\
+\n\
+Most targets do not need to implement this hook.  The default returns\n\
+@var{x} directly.  One target that does need to implement it is AArch64.\n\
+AArch64 needs to set the LSB of labels depending on the processor state.",
+ rtx, (rtx x),
+ hook_rtx_rtx_identity)
+
 /* Support for named address spaces.  */
 #undef HOOK_PREFIX
 #define HOOK_PREFIX "TARGET_ADDR_SPACE_"
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/label-addressing-includes-lsb.c b/gcc/testsuite/gcc.target/aarch64/morello/label-addressing-includes-lsb.c
new file mode 100644
index 00000000000..2c4a9b27d13
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/label-addressing-includes-lsb.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* This testcase ensures:
+   1) When a label is stored in the constant pool, it is stored using its
+      containing function's permissions and with the LSB set.
+   2) When a label is loaded directly in code (using adrp and add on the label
+      symbol directly) it has its LSB set.  */
+int fun(int x)
+{
+    void *a[] = {&&label, &&label2};
+    label:
+        goto *a[x++];
+    label2:
+        return x;
+}
+void* fun2(int x)
+{
+    void *a = &&label;
+label:
+    if (x % 10)
+      return a;
+    else
+      {
+label2:
+      return &&label2; /* { dg-warning "returns address of label" } */
+      }
+}
+
+/* Ensure that we initialise labels in the constant pool with the correct form.  */
+/* { dg-final { scan-assembler {capinit\tfun\+\(\(\.L\d\+1\)-fun\)} { target cheri_capability_pure } } } */
+
+/* Ensure that we load labels directly with the LSB set.  */
+/* { dg-final { scan-assembler {adrp[^\n]*\.L\d\+1} { target cheri_capability_pure } } } */