public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] Convert GCC-internal unwind_word to scalar_addr_mode
@ 2022-03-14 10:36 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2022-03-14 10:36 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:a227984b0990e455b47a8184a4e932649b3d91d9

commit a227984b0990e455b47a8184a4e932649b3d91d9
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Mon Mar 7 18:04:47 2022 +0000

    Convert GCC-internal unwind_word to scalar_addr_mode
    
    Now we've switched the definition of _Unwind_Word in the unwinder to
    represent a capability on capability architectures, it makes sense to
    change the `unwind_word` mode in GCC itself to mean a capability mode on
    capability architectures.
    
    The propagation of this means:
     - targetm.unwind_word_mode and targetm.eh_return_filter_mode have their
       return type changed to scalar_addr_mode.  Similarly their default
       implementations have the same change.
     - We add a new implementation of this hook for AArch64 that returns
       Pmode.
     - In order to implement the _Unwind_WordAddr type in libgcc, we
       introduce a new unwind_word_addr mode attribute that indicates the
       offset_mode of unwind_word.
     - This new unwind_word_addr mode attribute is then used in libgcc.
    
    If we want to define the `unwind_word` mode as a capability mode when
    that's what's needed for `_Unwind_Word`, then disallowing the use of
    `attribute((mode(unwind_word)))` on integers (which was originally done
    in commit cfc5512ea6ccb) breaks the main motivating case for this mode
    definition.
    
    This patch also allows the use of capability modes in a mode attribute
    on integers to define an INTCAP_TYPE.  This does allow a slightly
    confusing syntax of defining an INTCAP_TYPE based on a modified unsigned
    integer, but it means the definition of `unwind_word` and `_Unwind_Word`
    is consistent throughout the compiler and libgcc.
    
    That had the extra benefit that we no longer used __UINTPTR_TYPE__ for
    _Unwind_Ptr.  The uintptr_t value is guaranteed to be able to hold any
    value of a `void *`, but it's not guaranteed to be the same size as a
    `void *`.
    
    The existing implementation (before Morello changes) defined _Unwind_Ptr
    with a typedef using `__attribute__((__mode__(__pointer__)))` rather
    than with `__UINTPTR_TYPE__`.  This *is* guaranteed to be the same size
    as a `void *`.
    
    Since the unwinder reads in pointers from DWARF information, and this
    information is ABI defined to have the same size as a `void *`, we can
    not guarantee that changing to `__UINTPTR_TYPE__` will always work -- it
    may read in extra data from the dwarf information that would end up
    having a different value than the `void *` stored there.
    
    Being able to revert to `__attribute__((__mode__(__pointer__)))` avoids
    the above problem.
    
    During implementation we now ensure that `build_intcap_type_for_mode`
    caches the types that it can return (similar to how
    `build_pointer_type_for_mode` also caches the types that it can return).
    
    We needed that behaviour for this change because we use
    `lang_hooks.types.type_for_mode` in `handle_mode_attribute` to set the
    type node which we use after the application of the relevant mode
    attribute.
    
    As it stands, `lang_hooks.types.type_for_mode` uses
    `build_intcap_type_for_mode`, and since that function builds a new type
    each time we end up with differing types representing the same thing.
    Those differing types cause problems because they compare as different
    and hence type-handling code does the wrong thing on them.
    
    In this particular case we saw an ICE trying to find the common type for
    an expression because the types were not comparing equal and we had no
    way to handle two INTCAP_TYPE's with different `TYPE_MAIN_VARIANT`
    types.  Not having such a way makes sense because there are not
    `INTCAP_TYPE`'s with differing `TYPE_MAIN_VARIANT`'s in any capability
    architecture as yet.
    
    N.b. Because we now return a capability from `__builtin_extend_pointer`
    the testcase `aarch64/asm-4.c` now works as it did for non-capability.

Diff:
---
 gcc/c-family/c-attribs.c                           |  6 ++-
 gcc/c-family/c-common.c                            |  2 +-
 gcc/config/aarch64/aarch64.c                       | 10 +++++
 gcc/doc/tm.texi                                    |  2 +-
 gcc/except.c                                       |  6 +--
 gcc/target.def                                     |  4 +-
 gcc/targhooks.c                                    |  4 +-
 gcc/targhooks.h                                    |  4 +-
 gcc/testsuite/gcc.target/aarch64/asm-4.c           |  5 +--
 .../aarch64/morello/mode-attribute-disallow.c      |  6 ---
 gcc/tree.c                                         | 48 +++++++++++++++++++++-
 libgcc/unwind-dw2-fde.h                            |  6 ++-
 libgcc/unwind-generic.h                            |  9 +---
 13 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 20219b55ddc..7ab3e41f626 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -1853,6 +1853,8 @@ handle_mode_attribute (tree *node, tree name, tree args,
 	mode = targetm.libgcc_shift_count_mode ();
       else if (!strcmp (p, "unwind_word"))
 	mode = targetm.unwind_word_mode ();
+      else if (!strcmp (p, "unwind_word_addr"))
+	mode = offset_mode (targetm.unwind_word_mode ());
       else
 	for (j = 0; j < NUM_MACHINE_MODES; j++)
 	  if (!strcmp (p, GET_MODE_NAME (j)))
@@ -1993,7 +1995,9 @@ handle_mode_attribute (tree *node, tree name, tree args,
 	}
       else if (VECTOR_MODE_P (mode)
 	       ? TREE_CODE (type) != TREE_CODE (TREE_TYPE (typefm))
-	       : TREE_CODE (type) != TREE_CODE (typefm))
+	       : TREE_CODE (type) != TREE_CODE (typefm)
+	       && ! (TREE_CODE (type) == INTEGER_TYPE
+		     && INTCAP_TYPE_P (typefm)))
 	{
 	  error ("mode %qs applied to inappropriate type", p);
 	  return NULL_TREE;
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index dd0ded25d9d..650b1c94610 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -2266,7 +2266,7 @@ c_common_type_for_mode (machine_mode mode, int unsignedp)
       else
 	{
 	  gcc_assert (is_a <scalar_addr_mode> (mode));
-	  return unsignedp ? uintcap_type_node : intcap_type_node;
+	  return build_intcap_type_for_mode (mode, unsignedp);
 	}
     }
 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d3a9017ab3e..c0cf20cbea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7422,6 +7422,13 @@ aarch64_layout_frame (void)
   frame.laid_out = true;
 }
 
+/* Implementation of TARGET_UNWIND_WORD_MODE.  */
+scalar_addr_mode
+aarch64_unwind_word_mode (void)
+{
+  return Pmode;
+}
+
 /* Return true if the register REGNO is saved on entry to
    the current function.  */
 
@@ -25375,6 +25382,9 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_ASM_DECLARE_CONSTANT_NAME
 #define TARGET_ASM_DECLARE_CONSTANT_NAME aarch64_declare_constant_name
 
+#undef TARGET_UNWIND_WORD_MODE
+#define TARGET_UNWIND_WORD_MODE		aarch64_unwind_word_mode
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-aarch64.h"
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 019a2dc7fc6..97fe7eb371b 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -1421,7 +1421,7 @@ of shift instructions expanded to libgcc calls.  If not defined
 targets.
 @end deftypefn
 
-@deftypefn {Target Hook} scalar_int_mode TARGET_UNWIND_WORD_MODE (void)
+@deftypefn {Target Hook} scalar_addr_mode TARGET_UNWIND_WORD_MODE (void)
 Return machine mode to be used for @code{_Unwind_Word} type.
 The default is to use @code{word_mode}.
 @end deftypefn
diff --git a/gcc/except.c b/gcc/except.c
index e8a53d3a755..9dc146c29e5 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -1293,8 +1293,8 @@ sjlj_emit_function_exit (void)
 static void
 sjlj_emit_dispatch_table (rtx_code_label *dispatch_label, int num_dispatch)
 {
-  scalar_int_mode unwind_word_mode = targetm.unwind_word_mode ();
-  scalar_int_mode filter_mode = targetm.eh_return_filter_mode ();
+  scalar_addr_mode unwind_word_mode = targetm.unwind_word_mode ();
+  scalar_addr_mode filter_mode = targetm.eh_return_filter_mode ();
   eh_landing_pad lp;
   rtx mem, fc, exc_ptr_reg, filter_reg;
   rtx_insn *seq;
@@ -2120,7 +2120,7 @@ expand_builtin_eh_copy_values (tree exp)
     = expand_builtin_eh_common (CALL_EXPR_ARG (exp, 0));
   eh_region src
     = expand_builtin_eh_common (CALL_EXPR_ARG (exp, 1));
-  scalar_int_mode fmode = targetm.eh_return_filter_mode ();
+  scalar_addr_mode fmode = targetm.eh_return_filter_mode ();
 
   if (dst->exc_ptr_reg == NULL)
     dst->exc_ptr_reg = gen_reg_rtx (ptr_mode);
diff --git a/gcc/target.def b/gcc/target.def
index a5960e6f8d2..c5dd5264fad 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2159,7 +2159,7 @@ be sealed from modification without invalidating the pointer.",
 DEFHOOK_UNDOC
 (eh_return_filter_mode,
  "Return machine mode for filter value.",
- scalar_int_mode, (void),
+ scalar_addr_mode, (void),
  default_eh_return_filter_mode)
 
 /* Return machine mode for libgcc expanded cmp instructions.  */
@@ -2187,7 +2187,7 @@ DEFHOOK
 (unwind_word_mode,
  "Return machine mode to be used for @code{_Unwind_Word} type.\n\
 The default is to use @code{word_mode}.",
- scalar_int_mode, (void),
+ scalar_addr_mode, (void),
  default_unwind_word_mode)
 
 /* Given two decls, merge their attributes and return the result.  */
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 19146dfc6db..129c3eec3fd 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -226,7 +226,7 @@ default_pretend_outgoing_varargs_named (cumulative_args_t ca ATTRIBUTE_UNUSED)
 	  != default_setup_incoming_varargs);
 }
 
-scalar_int_mode
+scalar_addr_mode
 default_eh_return_filter_mode (void)
 {
   return targetm.unwind_word_mode ();
@@ -244,7 +244,7 @@ default_libgcc_shift_count_mode (void)
   return word_mode;
 }
 
-scalar_int_mode
+scalar_addr_mode
 default_unwind_word_mode (void)
 {
   return word_mode;
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 43ce9026981..dd58c437339 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -47,10 +47,10 @@ extern void default_handle_outgoing_varargs (rtx, int, rtx *);
 extern rtx default_builtin_setjmp_frame_value (void);
 extern bool default_pretend_outgoing_varargs_named (cumulative_args_t);
 
-extern scalar_int_mode default_eh_return_filter_mode (void);
+extern scalar_addr_mode default_eh_return_filter_mode (void);
 extern scalar_int_mode default_libgcc_cmp_return_mode (void);
 extern scalar_int_mode default_libgcc_shift_count_mode (void);
-extern scalar_int_mode default_unwind_word_mode (void);
+extern scalar_addr_mode default_unwind_word_mode (void);
 extern unsigned HOST_WIDE_INT default_shift_truncation_mask
   (machine_mode);
 extern unsigned int default_min_divisions_for_recip_mul (machine_mode);
diff --git a/gcc/testsuite/gcc.target/aarch64/asm-4.c b/gcc/testsuite/gcc.target/aarch64/asm-4.c
index c6f3e6c88c0..abe2af5f1d1 100644
--- a/gcc/testsuite/gcc.target/aarch64/asm-4.c
+++ b/gcc/testsuite/gcc.target/aarch64/asm-4.c
@@ -1,4 +1,4 @@
-/* { dg-do compile { xfail aarch64_capability_any } } */
+/* { dg-do compile } */
 /* { dg-options "-O0" } */
 
 int x;
@@ -6,6 +6,5 @@ int x;
 void
 f (void)
 {
-  asm volatile ("%a0" :: "X" (__builtin_extend_pointer (&x))); /* { dg-bogus "invalid address mode" "" { xfail aarch64_capability_any } } */
-  /* { dg-bogus "invalid expression as operand" "" { xfail aarch64_capability_any } .-1 } */
+  asm volatile ("%a0" :: "X" (__builtin_extend_pointer (&x)));
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/mode-attribute-disallow.c b/gcc/testsuite/gcc.target/aarch64/morello/mode-attribute-disallow.c
deleted file mode 100644
index 96b44aef61f..00000000000
--- a/gcc/testsuite/gcc.target/aarch64/morello/mode-attribute-disallow.c
+++ /dev/null
@@ -1,6 +0,0 @@
-/* { dg-do compile } */
-/* TODO Ensure only ran with -mfake-capability or with purecap. */
-
-typedef unsigned myval __attribute__((__mode__(__pointer__))); /* { dg-error "mode 'pointer' applied to inappropriate type" "pointer mode attribute disallowed for unsigned" } */
-/* TODO Check that __mode__(address) works and provides the offset mode of
-   pointers.  */
diff --git a/gcc/tree.c b/gcc/tree.c
index 982f198fd95..98d5cebfa4f 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -7902,6 +7902,24 @@ tree
 build_intcap_type_for_mode (machine_mode mode, int unsignedp)
 {
   gcc_assert (CAPABILITY_MODE_P (mode));
+
+  /* There is currently no target which has more than one capability mode.
+     Because of this, cacheing INTCAP_TYPE's is easy, there are only two kinds.
+     If/when a target with more than one capability mode is built this cacheing
+     mechanism should trigger an ICE and be easy enough to spot as a change to
+     make.
+     We need to cache since otherwise comparisons of types don't work as
+     expected (since we check the type pointer itself).  */
+  if (unsignedp && uintcap_type_node != NULL)
+    {
+      gcc_assert (TYPE_MODE (uintcap_type_node) == mode);
+      return uintcap_type_node;
+    }
+  if (!unsignedp && intcap_type_node != NULL)
+    {
+      gcc_assert (TYPE_MODE (intcap_type_node) == mode);
+      return intcap_type_node;
+    }
   tree t = make_node (INTCAP_TYPE);
   SET_TYPE_MODE (t, mode);
   TYPE_UNSIGNED (t) = !!unsignedp;
@@ -7922,6 +7940,25 @@ build_intcap_type_for_mode (machine_mode mode, int unsignedp)
   set_min_and_max_values_for_integral_type
 	  (t, TYPE_CAP_PRECISION (t), unsignedp ? UNSIGNED : SIGNED);
 
+  /* N.b. this is somewhat superfluous, since the time this function is called
+     from `build_common_tree_nodes` will be the first time this function is
+     called (since that is called from the language initialisation code), and
+     `build_common_tree_nodes` assigns the result of this function to the
+     global_trees elements `uintcap_type_node` and `intcap_type_node`.
+
+     That said, it's nice to have the cacheing in this function itself rather
+     than outside of it since otherwise this function has unnecessary
+     dependencies on how it gets called.
+     Given that, and the fact that there's no harm in the superfluous
+     double-assignment we simply ensure the assignment is done in this function
+     and in `build_common_tree_nodes`.  The assignment in
+     `build_common_tree_nodes` makes it clearer what's happening there, so we
+     leave it.  */
+  if (unsignedp)
+    uintcap_type_node = t;
+  else
+    intcap_type_node = t;
+
   return t;
 }
 
@@ -10378,8 +10415,15 @@ build_common_tree_nodes (bool signed_char)
   if (opt_cap_mode.exists())
     {
       scalar_addr_mode cap_mode = opt_cap_mode.require();
-      intcap_type_node = build_intcap_type_for_mode (cap_mode, 0);
-      uintcap_type_node = build_intcap_type_for_mode (cap_mode, 1);
+      /* `build_intcap_type_for_mode` caches its return values in
+	 `uintcap_type_node` and `intcap_type_node`, so we don't need to do the
+	 assignment here.  We *do* need to ensure that those nodes are set by
+	 the time this function returns, since they are used by other code
+	 without calling `build_intcap_type_for_mode`.  */
+      gcc_assert (build_intcap_type_for_mode (cap_mode, 0)
+		  == intcap_type_node);
+      gcc_assert (build_intcap_type_for_mode (cap_mode, 1)
+		  == uintcap_type_node);
     }
 
   /* Fill in the rest of the sized types.  Reuse existing type nodes
diff --git a/libgcc/unwind-dw2-fde.h b/libgcc/unwind-dw2-fde.h
index c5fe25fc305..903ac989e0f 100644
--- a/libgcc/unwind-dw2-fde.h
+++ b/libgcc/unwind-dw2-fde.h
@@ -164,8 +164,10 @@ next_fde (const fde *f)
 }
 
 typedef unsigned _Unwind_Address __attribute__((__mode__(__address__)));
-typedef unsigned _Unwind_WordAddr __attribute__((__mode__(__unwind_word__)));
-typedef signed _Unwind_SwordAddr __attribute__((__mode__(__unwind_word__)));
+typedef unsigned _Unwind_WordAddr
+      __attribute__((__mode__(__unwind_word_addr__)));
+typedef signed _Unwind_SwordAddr
+      __attribute__((__mode__(__unwind_word_addr__)));
 extern const fde * _Unwind_Find_FDE (void *, struct dwarf_eh_bases *);
 
 static inline int
diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h
index 74268462cc7..d4eefa33323 100644
--- a/libgcc/unwind-generic.h
+++ b/libgcc/unwind-generic.h
@@ -45,22 +45,17 @@ extern "C" {
 
 /* @@@ The IA-64 ABI uses uint64 throughout.  Most places this is
    inefficient for 32-bit and smaller machines.  */
-#ifdef __CHERI_PURE_CAPABILITY__
-typedef __UINTPTR_TYPE__ _Unwind_Word;
-typedef __INTPTR_TYPE__ _Unwind_Sword;
-#else
 typedef unsigned _Unwind_Word __attribute__((__mode__(__unwind_word__)));
 typedef signed _Unwind_Sword __attribute__((__mode__(__unwind_word__)));
-#endif
 
 #if defined(__ia64__) && defined(__hpux__)
 typedef unsigned _Unwind_Ptr __attribute__((__mode__(__word__)));
 typedef unsigned _Unwind_Address __attribute__((__mode__(__word__)));
 #else
-typedef __UINTPTR_TYPE__ _Unwind_Ptr;
 typedef unsigned _Unwind_Address __attribute__((__mode__(__address__)));
+typedef unsigned _Unwind_Ptr __attribute__((__mode__(__pointer__)));
 #endif
-typedef __UINTPTR_TYPE__ _Unwind_Internal_Ptr;
+typedef unsigned _Unwind_Internal_Ptr __attribute__((__mode__(__pointer__)));
 
 /* @@@ The IA-64 ABI uses a 64-bit word to identify the producer and
    consumer of an exception.  We'll go along with this for now even on


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-03-14 10:36 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 10:36 [gcc(refs/vendors/ARM/heads/morello)] Convert GCC-internal unwind_word to scalar_addr_mode Matthew Malcomson

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