public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Christophe Lyon <christophe.lyon@linaro.org>
To: gcc-patches@gcc.gnu.org, Kyrylo.Tkachov@arm.com,
	richard.sandiford@arm.com
Cc: Christophe Lyon <christophe.lyon@linaro.org>
Subject: [PATCH] arm: Fix MVE intrinsics support with LTO (PR target/110268)
Date: Mon, 26 Jun 2023 15:02:30 +0000	[thread overview]
Message-ID: <20230626150230.2483991-1-christophe.lyon@linaro.org> (raw)

After the recent MVE intrinsics re-implementation, LTO stopped working
because the intrinsics would no longer be defined.

The main part of the patch is simple and similar to what we do for
AArch64:
- call handle_arm_mve_h() from arm_init_mve_builtins to declare the
  intrinsics when the compiler is in LTO mode
- actually implement arm_builtin_decl for MVE.

It was just a bit tricky to handle __ARM_MVE_PRESERVE_USER_NAMESPACE:
its value in the user code cannot be guessed at LTO time, so we always
have to assume that it was not defined.  The led to a few fixes in the
way we register MVE builtins as placeholders or not.  Without this
patch, we would just omit some versions of the inttrinsics when
__ARM_MVE_PRESERVE_USER_NAMESPACE is true. In fact, like for the C/C++
placeholders, we need to always keep entries for all of them to ensure
that we have a consistent numbering scheme.

	2023-06-26  Christophe Lyon   <christophe.lyon@linaro.org>

	PR target/110268
	gcc/
	* config/arm/arm-builtins.cc (arm_init_mve_builtins): Handle LTO.
	(arm_builtin_decl): Hahndle MVE builtins.
	* config/arm/arm-mve-builtins.cc (builtin_decl): New function.
	(add_unique_function): Fix handling of
	__ARM_MVE_PRESERVE_USER_NAMESPACE.
	(add_overloaded_function): Likewise.
	* config/arm/arm-protos.h (builtin_decl): New declaration.

	gcc/testsuite/
	* gcc.target/arm/pr110268-1.c: New test.
	* gcc.target/arm/pr110268-2.c: New test.
---
 gcc/config/arm/arm-builtins.cc            | 11 +++-
 gcc/config/arm/arm-mve-builtins.cc        | 61 ++++++++++++-----------
 gcc/config/arm/arm-protos.h               |  1 +
 gcc/testsuite/gcc.target/arm/pr110268-1.c | 11 ++++
 gcc/testsuite/gcc.target/arm/pr110268-2.c | 22 ++++++++
 5 files changed, 76 insertions(+), 30 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/pr110268-1.c
 create mode 100644 gcc/testsuite/gcc.target/arm/pr110268-2.c

diff --git a/gcc/config/arm/arm-builtins.cc b/gcc/config/arm/arm-builtins.cc
index 36365e40a5b..fca7dcaf565 100644
--- a/gcc/config/arm/arm-builtins.cc
+++ b/gcc/config/arm/arm-builtins.cc
@@ -1918,6 +1918,15 @@ arm_init_mve_builtins (void)
       arm_builtin_datum *d = &mve_builtin_data[i];
       arm_init_builtin (fcode, d, "__builtin_mve");
     }
+
+  if (in_lto_p)
+    {
+      arm_mve::handle_arm_mve_types_h ();
+      /* Under LTO, we cannot know whether
+	 __ARM_MVE_PRESERVE_USER_NAMESPACE was defined, so assume it
+	 was not.  */
+      arm_mve::handle_arm_mve_h (false);
+    }
 }
 
 /* Set up all the NEON builtins, even builtins for instructions that are not
@@ -2723,7 +2732,7 @@ arm_builtin_decl (unsigned code, bool initialize_p ATTRIBUTE_UNUSED)
     case ARM_BUILTIN_GENERAL:
       return arm_general_builtin_decl (subcode);
     case ARM_BUILTIN_MVE:
-      return error_mark_node;
+      return arm_mve::builtin_decl (subcode);
     default:
       gcc_unreachable ();
     }
diff --git a/gcc/config/arm/arm-mve-builtins.cc b/gcc/config/arm/arm-mve-builtins.cc
index 7033e41a571..e9a12f27411 100644
--- a/gcc/config/arm/arm-mve-builtins.cc
+++ b/gcc/config/arm/arm-mve-builtins.cc
@@ -493,6 +493,16 @@ handle_arm_mve_h (bool preserve_user_namespace)
 				     preserve_user_namespace);
 }
 
+/* Return the function decl with SVE function subcode CODE, or error_mark_node
+   if no such function exists.  */
+tree
+builtin_decl (unsigned int code)
+{
+  if (code >= vec_safe_length (registered_functions))
+    return error_mark_node;
+  return (*registered_functions)[code]->decl;
+}
+
 /* Return true if CANDIDATE is equivalent to MODEL_TYPE for overloading
    purposes.  */
 static bool
@@ -849,7 +859,6 @@ function_builder::add_function (const function_instance &instance,
     ? integer_zero_node
     : simulate_builtin_function_decl (input_location, name, fntype,
 				      code, NULL, attrs);
-
   registered_function &rfn = *ggc_alloc <registered_function> ();
   rfn.instance = instance;
   rfn.decl = decl;
@@ -889,15 +898,12 @@ function_builder::add_unique_function (const function_instance &instance,
   gcc_assert (!*rfn_slot);
   *rfn_slot = &rfn;
 
-  /* Also add the non-prefixed non-overloaded function, if the user namespace
-     does not need to be preserved.  */
-  if (!preserve_user_namespace)
-    {
-      char *noprefix_name = get_name (instance, false, false);
-      tree attrs = get_attributes (instance);
-      add_function (instance, noprefix_name, fntype, attrs, requires_float,
-		    false, false);
-    }
+  /* Also add the non-prefixed non-overloaded function, as placeholder
+     if the user namespace does not need to be preserved.  */
+  char *noprefix_name = get_name (instance, false, false);
+  attrs = get_attributes (instance);
+  add_function (instance, noprefix_name, fntype, attrs, requires_float,
+		false, preserve_user_namespace);
 
   /* Also add the function under its overloaded alias, if we want
      a separate decl for each instance of an overloaded function.  */
@@ -905,20 +911,17 @@ function_builder::add_unique_function (const function_instance &instance,
   if (strcmp (name, overload_name) != 0)
     {
       /* Attribute lists shouldn't be shared.  */
-      tree attrs = get_attributes (instance);
+      attrs = get_attributes (instance);
       bool placeholder_p = !(m_direct_overloads || force_direct_overloads);
       add_function (instance, overload_name, fntype, attrs,
 		    requires_float, false, placeholder_p);
 
-      /* Also add the non-prefixed overloaded function, if the user namespace
-	 does not need to be preserved.  */
-      if (!preserve_user_namespace)
-	{
-	  char *noprefix_overload_name = get_name (instance, false, true);
-	  tree attrs = get_attributes (instance);
-	  add_function (instance, noprefix_overload_name, fntype, attrs,
-			requires_float, false, placeholder_p);
-	}
+      /* Also add the non-prefixed overloaded function, as placeholder
+	 if the user namespace does not need to be preserved.  */
+      char *noprefix_overload_name = get_name (instance, false, true);
+      attrs = get_attributes (instance);
+      add_function (instance, noprefix_overload_name, fntype, attrs,
+		    requires_float, false, preserve_user_namespace || placeholder_p);
     }
 
   obstack_free (&m_string_obstack, name);
@@ -948,15 +951,15 @@ function_builder::add_overloaded_function (const function_instance &instance,
 	= add_function (instance, name, m_overload_type, NULL_TREE,
 			requires_float, true, m_direct_overloads);
       m_overload_names.put (name, &rfn);
-      if (!preserve_user_namespace)
-	{
-	  char *noprefix_name = get_name (instance, false, true);
-	  registered_function &noprefix_rfn
-	    = add_function (instance, noprefix_name, m_overload_type,
-			    NULL_TREE, requires_float, true,
-			    m_direct_overloads);
-	  m_overload_names.put (noprefix_name, &noprefix_rfn);
-	}
+
+      /* Also add the non-prefixed function, as placeholder if the
+	 user namespace does not need to be preserved.  */
+      char *noprefix_name = get_name (instance, false, true);
+      registered_function &noprefix_rfn
+	= add_function (instance, noprefix_name, m_overload_type,
+			NULL_TREE, requires_float, true,
+			preserve_user_namespace || m_direct_overloads);
+      m_overload_names.put (noprefix_name, &noprefix_rfn);
     }
 }
 
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 7d73c66a15d..6186921011e 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -232,6 +232,7 @@ const unsigned int ARM_BUILTIN_CLASS = (1 << ARM_BUILTIN_SHIFT) - 1;
 namespace arm_mve {
   void handle_arm_mve_types_h ();
   void handle_arm_mve_h (bool);
+  tree builtin_decl (unsigned);
   tree resolve_overloaded_builtin (location_t, unsigned int,
 				   vec<tree, va_gc> *);
   bool check_builtin_call (location_t, vec<location_t>, unsigned int,
diff --git a/gcc/testsuite/gcc.target/arm/pr110268-1.c b/gcc/testsuite/gcc.target/arm/pr110268-1.c
new file mode 100644
index 00000000000..d133011343c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr110268-1.c
@@ -0,0 +1,11 @@
+/* { dg-do link }  */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -flto -specs=rdimon.specs" } */
+
+#include <arm_mve.h>
+
+int main(int argc, char* argv[])
+{
+  return vaddvq(__arm_vdupq_n_s8 (argc));
+}
diff --git a/gcc/testsuite/gcc.target/arm/pr110268-2.c b/gcc/testsuite/gcc.target/arm/pr110268-2.c
new file mode 100644
index 00000000000..ad03fb37793
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr110268-2.c
@@ -0,0 +1,22 @@
+/* { dg-do link }  */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -flto -specs=rdimon.specs" } */
+
+/* Check MVE intrinsics with LTO with __ARM_MVE_PRESERVE_USER_NAMESPACE and a
+   user-overridden intrinsic.  */
+
+#define __ARM_MVE_PRESERVE_USER_NAMESPACE
+#include <arm_mve.h>
+
+int global_int;
+int32_t vaddvq(int8x16_t x)
+{
+  return global_int + __arm_vgetq_lane_s8 (x, 0);
+}
+
+int main(int argc, char* argv[])
+{
+  global_int = argc;
+  return vaddvq(__arm_vdupq_n_s8 (argc));
+}
-- 
2.34.1


             reply	other threads:[~2023-06-26 15:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-26 15:02 Christophe Lyon [this message]
2023-06-26 15:29 ` Prathamesh Kulkarni
2023-06-26 15:32   ` Christophe Lyon
2023-07-03  7:44 ` Christophe Lyon
2023-07-05 17:07 ` Kyrylo Tkachov
2023-07-06 15:20   ` Christophe Lyon
2023-07-06 16:38     ` Kyrylo Tkachov
2023-07-10 13:08       ` [PATCH v2] " Christophe Lyon
2023-07-10 13:48         ` Kyrylo Tkachov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230626150230.2483991-1-christophe.lyon@linaro.org \
    --to=christophe.lyon@linaro.org \
    --cc=Kyrylo.Tkachov@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).