diff mbox series

[v3,1/1] CPP: Add flag to generate resolver at default version implementation.

Message ID 20241126143503.2897121-2-alfie.richards@arm.com
State New
Headers
Series CPP: Add flag to generate resolver at default version implementation. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Alfie Richards Nov. 26, 2024, 2:35 p.m. UTC 
  This patch adds the TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL flag
which changes FMV behavior for target_version functions to match the Arm
C Language Extension.

The functional differences consist of:
1. Generating the resolver for the dispatched symbol at the site of the
   default version definition.
2. Mangling non-default FMV annotated functions even when no other
   versions are present.

This allows for better behavior when definitions are spread across
different TU's as one resolver will be created and the
[ACLE](https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning)
stipulates that the default implementation must have visibility of all
other versions so that resolver will be complete. This also matches
Clangs behavior.

The only remaining discrepancy I'm aware of when targeting AArch64 is we
do not allow the coexistence of target_version and target_clone, which
is specified as supported in the ACLE but will be addressed later.

Note this hook is only true on Aarch64, so other targets will not be
affected.

To enable these functionality changes I added mangling to the initial
processing of functions in the CPP frontend, and changed the
logic for the creation of resolver bodies to create at default
declaration implementations.

Additionally, the previous naming logic relied on the fact that if there
existed a call to a dispatched function the resolver would also be
created which would do some renaming. As that no longer is guaranteed
this patch hacks on the assembler names to make them correct.

Reg tested on AArch64 and X86_64.
Bootstrapped tested on aarch64-none-linux-gnu and
x86_64-unknown-linux-gnu.

gcc/ChangeLog:

	* attribs.cc (make_dispatcher_decl): Add optional name parameter.
	(is_function_default_version): Add check_versioned parameter.
	* attribs.h (make_dispatcher_decl): Add optional name parameter.
	(is_function_default_version): Add optional check_versioned parameter.
	* cgraphunit.cc (cgraph_node::analyze): Change dispatcher creation logic.
	* config/aarch64/aarch64.cc (get_assembler_name_without_default): New.
	(get_suffixed_assembler_name): Change to use get_assembler_name_without_default.
	(aarch64_get_function_versions_dispatcher): Change to use get_assembler_name_without_default.
	* config/aarch64/aarch64.h (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL): New.
	* defaults.h (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL): New.

gcc/cp/ChangeLog:

	* class.cc (add_method): Update call
	* cp-tree.h (maybe_version_functions): Remove unecessary argument.
	* decl.cc (decls_match): Remove unecessary argument.
	(maybe_version_functions): Remove unecessary parameter.
	(start_preparsed_function): Change logic.

gcc/testsuite/ChangeLog:

	* g++.target/aarch64/mv-1.C: Update.
	* g++.target/aarch64/mv-symbols2.C: Update.
	* g++.target/aarch64/mv-symbols3.C: Update.
	* g++.target/aarch64/mv-symbols4.C: Update.
	* g++.target/aarch64/mv-symbols5.C: Update.
	* g++.target/aarch64/mv-symbols6.C: New test.
---
 gcc/attribs.cc                                | 31 +++++++++++-----
 gcc/attribs.h                                 |  6 ++--
 gcc/cgraphunit.cc                             | 36 +++++++++++++++----
 gcc/config/aarch64/aarch64.cc                 | 31 ++++++++++++----
 gcc/config/aarch64/aarch64.h                  |  2 ++
 gcc/cp/class.cc                               |  2 +-
 gcc/cp/cp-tree.h                              |  2 +-
 gcc/cp/decl.cc                                | 18 ++++++----
 gcc/defaults.h                                | 10 ++++++
 gcc/testsuite/g++.target/aarch64/mv-1.C       |  4 +++
 .../g++.target/aarch64/mv-symbols2.C          | 12 +++----
 .../g++.target/aarch64/mv-symbols3.C          |  6 ++--
 .../g++.target/aarch64/mv-symbols4.C          |  6 ++--
 .../g++.target/aarch64/mv-symbols5.C          |  6 ++--
 .../g++.target/aarch64/mv-symbols6.C          | 22 ++++++++++++
 15 files changed, 148 insertions(+), 46 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/aarch64/mv-symbols6.C

Comments

Alfie Richards Dec. 3, 2024, 10:01 a.m. UTC | #1 
Hi all,

Ping for this as it no longer needs the C patch and is independent.

Kind regards,
Alfie

On 26/11/2024 14:35, alfie.richards@arm.com wrote:
> This patch adds the TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL flag
> which changes FMV behavior for target_version functions to match the Arm
> C Language Extension.
>
> The functional differences consist of:
> 1. Generating the resolver for the dispatched symbol at the site of the
>     default version definition.
> 2. Mangling non-default FMV annotated functions even when no other
>     versions are present.
>
> This allows for better behavior when definitions are spread across
> different TU's as one resolver will be created and the
> [ACLE](https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning)
> stipulates that the default implementation must have visibility of all
> other versions so that resolver will be complete. This also matches
> Clangs behavior.
>
> The only remaining discrepancy I'm aware of when targeting AArch64 is we
> do not allow the coexistence of target_version and target_clone, which
> is specified as supported in the ACLE but will be addressed later.
>
> Note this hook is only true on Aarch64, so other targets will not be
> affected.
>
> To enable these functionality changes I added mangling to the initial
> processing of functions in the CPP frontend, and changed the
> logic for the creation of resolver bodies to create at default
> declaration implementations.
>
> Additionally, the previous naming logic relied on the fact that if there
> existed a call to a dispatched function the resolver would also be
> created which would do some renaming. As that no longer is guaranteed
> this patch hacks on the assembler names to make them correct.
>
> Reg tested on AArch64 and X86_64.
> Bootstrapped tested on aarch64-none-linux-gnu and
> x86_64-unknown-linux-gnu.
>
> gcc/ChangeLog:
>
> 	* attribs.cc (make_dispatcher_decl): Add optional name parameter.
> 	(is_function_default_version): Add check_versioned parameter.
> 	* attribs.h (make_dispatcher_decl): Add optional name parameter.
> 	(is_function_default_version): Add optional check_versioned parameter.
> 	* cgraphunit.cc (cgraph_node::analyze): Change dispatcher creation logic.
> 	* config/aarch64/aarch64.cc (get_assembler_name_without_default): New.
> 	(get_suffixed_assembler_name): Change to use get_assembler_name_without_default.
> 	(aarch64_get_function_versions_dispatcher): Change to use get_assembler_name_without_default.
> 	* config/aarch64/aarch64.h (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL): New.
> 	* defaults.h (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL): New.
>
> gcc/cp/ChangeLog:
>
> 	* class.cc (add_method): Update call
> 	* cp-tree.h (maybe_version_functions): Remove unecessary argument.
> 	* decl.cc (decls_match): Remove unecessary argument.
> 	(maybe_version_functions): Remove unecessary parameter.
> 	(start_preparsed_function): Change logic.
>
> gcc/testsuite/ChangeLog:
>
> 	* g++.target/aarch64/mv-1.C: Update.
> 	* g++.target/aarch64/mv-symbols2.C: Update.
> 	* g++.target/aarch64/mv-symbols3.C: Update.
> 	* g++.target/aarch64/mv-symbols4.C: Update.
> 	* g++.target/aarch64/mv-symbols5.C: Update.
> 	* g++.target/aarch64/mv-symbols6.C: New test.
> ---
>   gcc/attribs.cc                                | 31 +++++++++++-----
>   gcc/attribs.h                                 |  6 ++--
>   gcc/cgraphunit.cc                             | 36 +++++++++++++++----
>   gcc/config/aarch64/aarch64.cc                 | 31 ++++++++++++----
>   gcc/config/aarch64/aarch64.h                  |  2 ++
>   gcc/cp/class.cc                               |  2 +-
>   gcc/cp/cp-tree.h                              |  2 +-
>   gcc/cp/decl.cc                                | 18 ++++++----
>   gcc/defaults.h                                | 10 ++++++
>   gcc/testsuite/g++.target/aarch64/mv-1.C       |  4 +++
>   .../g++.target/aarch64/mv-symbols2.C          | 12 +++----
>   .../g++.target/aarch64/mv-symbols3.C          |  6 ++--
>   .../g++.target/aarch64/mv-symbols4.C          |  6 ++--
>   .../g++.target/aarch64/mv-symbols5.C          |  6 ++--
>   .../g++.target/aarch64/mv-symbols6.C          | 22 ++++++++++++
>   15 files changed, 148 insertions(+), 46 deletions(-)
>   create mode 100644 gcc/testsuite/g++.target/aarch64/mv-symbols6.C
>
Jason Merrill Dec. 3, 2024, 10:10 p.m. UTC | #2 
On 11/26/24 9:35 AM, alfie.richards@arm.com wrote:
> 
> This patch adds the TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL flag
> which changes FMV behavior for target_version functions to match the Arm
> C Language Extension.
> 
> The functional differences consist of:
> 1. Generating the resolver for the dispatched symbol at the site of the
>     default version definition.
> 2. Mangling non-default FMV annotated functions even when no other
>     versions are present.
> 
> This allows for better behavior when definitions are spread across
> different TU's as one resolver will be created and the
> [ACLE](https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning)
> stipulates that the default implementation must have visibility of all
> other versions so that resolver will be complete. This also matches
> Clangs behavior.
> 
> The only remaining discrepancy I'm aware of when targeting AArch64 is we
> do not allow the coexistence of target_version and target_clone, which
> is specified as supported in the ACLE but will be addressed later.
> 
> Note this hook is only true on Aarch64, so other targets will not be
> affected.
> 
> To enable these functionality changes I added mangling to the initial
> processing of functions in the CPP frontend, and changed the

Please don't refer to C++ as CPP, to me that means the C preprocessor.

> logic for the creation of resolver bodies to create at default
> declaration implementations.
> 
> Additionally, the previous naming logic relied on the fact that if there
> existed a call to a dispatched function the resolver would also be
> created which would do some renaming. As that no longer is guaranteed
> this patch hacks on the assembler names to make them correct.


> @@ -1250,13 +1250,17 @@ common_function_versions (tree fn1, tree fn2)
>     by the front-end.  Return the decl created.  */
>  
>  tree
> -make_dispatcher_decl (const tree decl)
> +make_dispatcher_decl (const tree decl, const char *name)

The function comment needs updating for the new parameter.

> +/* If allow_unversioned is false, returns true if DECL has been marked as
> +   multiversioned, is multi-versioned using the an attribute, and this is

"the an"?

We conventionally refer to parameter names in all caps (i.e. 
ALLOW_UNVERSIONED) in the function comment.

> +   the default version.
> +   If allow_unversioned is true, then does not require the DECL is marked as
> +   versioned and returns true if the function could be a default function,
> +   ie could be unannotated.  */

I think it would actually be clearer to leave the current first sentence 
alone other than changing "the target attribute" to "an attribute", and 
say that the function additionally returns true in this case if 
allow_unversioned.

Actually, why does this need to be a parameter at all?  Why not 
condition it on the target macro?

>  
>  bool
> -is_function_default_version (const tree decl)
> +is_function_default_version (const tree decl, bool allow_unversioned)

> +    = lookup_attribute (TARGET_HAS_FMV_TARGET_ATTRIBUTE ? "target"
> +							: "target_version",

That indentation won't last, let's move the ? to the second line as well.

> -extern tree make_dispatcher_decl (const tree);
> -extern bool is_function_default_version (const tree);
> +extern tree
> +make_dispatcher_decl (const tree, const char *name = NULL);
> +extern bool
> +is_function_default_version (const tree, bool = false);

We only put the function name at the beginning of the line for 
definitions, not forward declarations.

> +      /* Check for the default version.  */
> +      if (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
> +	  && !is_function_default_version (
> +	    function_version ()->next->this_node->decl, true)
> +	  && callers)

We don't put ( at the end of a line.

Why are we calling function_version () again here instead of referring 
to version_info?

> +      /* Generate the dispatcher body of multi-versioned functions at
> +	 the first point where the dispatched symbol has been called.  */
> +      if ((!TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
> +	   || (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL && version_info
> +	       && version_info->next
> +	       && is_function_default_version (
> +		 version_info->next->this_node->decl, true)

This is the exact same condition as above, let's use a local variable to 
store its value.

> +  /* We need to always process the dispatcher resolver in the
> +     TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL fmv case.  */
> +  if (version_info && TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
> +      && !dispatcher_function && is_function_default_version (decl, true))
> +    enqueue_node (cgraph_node::get_create (
> +      targetm.get_function_versions_dispatcher (decl)));

Again no ( at end of line.

> @@ -20364,6 +20364,18 @@ aarch64_mangle_decl_assembler_name (tree decl, tree id)
>    return id;
>  }
>  
> +static std::string
> +get_assembler_name_without_default (tree default_decl)
> +{
> +  std::string name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (default_decl));
> +
> +  auto size = name.size ();
> +  if (size >= 8 && name.compare (size - 8, 8, ".default") == 0)
> +    name.resize (size - 8);
> +
> +  return name;
> +}

This function needs a comment.

> @@ -20839,8 +20847,19 @@ aarch64_get_function_versions_dispatcher (void *decl)

> +      /* Mark the assembler name as set to prevent it getting mangled again .*/
> +      SET_DECL_ASSEMBLER_NAME (dispatch_decl,
> +			       DECL_ASSEMBLER_NAME (dispatch_decl));

This doesn't make sense to me.  When you refer to DECL_ASSEMBLER_NAME it 
gets set; passing the result to SET_DECL_ASSEMBLER_NAME shouldn't have 
any effect.  I think you can lose this whole statement.

> @@ -18409,6 +18406,15 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
>    if (!DECL_OMP_DECLARE_REDUCTION_P (decl1))
>      start_lambda_scope (decl1);
>  
> +  /* To enable versions to be created across TU's we mark and mangle all
> +     non-default versioned functions.  */
> +  if (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
> +      && lookup_attribute (TARGET_HAS_FMV_TARGET_ATTRIBUTE ? "target"
> +							   : "target_version",
> +			   DECL_ATTRIBUTES (decl1)))
> +    if (!is_function_default_version (decl1, true))
> +      maybe_mark_function_versioned (decl1);

The lookup_attribute seems redundant with !is_function_default_version.

> +   extension and esures batter behavior when defining function versions

"ensures better"

Jason
diff mbox series

Patch

diff --git a/gcc/attribs.cc b/gcc/attribs.cc
index 1d6589835a1..a8474db3b0a 100644
--- a/gcc/attribs.cc
+++ b/gcc/attribs.cc
@@ -1250,13 +1250,17 @@  common_function_versions (tree fn1, tree fn2)
    by the front-end.  Return the decl created.  */
 
 tree
-make_dispatcher_decl (const tree decl)
+make_dispatcher_decl (const tree decl, const char *name)
 {
   tree func_decl;
-  char *func_name;
   tree fn_type, func_type;
 
-  func_name = xstrdup (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
+  char *func_name;
+
+  if (name)
+    func_name = xstrdup (name);
+  else
+    func_name = xstrdup (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
 
   fn_type = TREE_TYPE (decl);
   func_type = build_function_type (TREE_TYPE (fn_type),
@@ -1277,17 +1281,26 @@  make_dispatcher_decl (const tree decl)
   return func_decl;
 }
 
-/* Returns true if DECL is multi-versioned using the target attribute, and this
-   is the default version.  This function can only be used for targets that do
-   not support the "target_version" attribute.  */
+/* If allow_unversioned is false, returns true if DECL has been marked as
+   multiversioned, is multi-versioned using the an attribute, and this is
+   the default version.
+   If allow_unversioned is true, then does not require the DECL is marked as
+   versioned and returns true if the function could be a default function,
+   ie could be unannotated.  */
 
 bool
-is_function_default_version (const tree decl)
+is_function_default_version (const tree decl, bool allow_unversioned)
 {
   if (TREE_CODE (decl) != FUNCTION_DECL
-      || !DECL_FUNCTION_VERSIONED (decl))
+      || (!allow_unversioned && !DECL_FUNCTION_VERSIONED (decl)))
     return false;
-  tree attr = lookup_attribute ("target", DECL_ATTRIBUTES (decl));
+  tree attr
+    = lookup_attribute (TARGET_HAS_FMV_TARGET_ATTRIBUTE ? "target"
+							: "target_version",
+			DECL_ATTRIBUTES (decl));
+  /* An unannotated function can be default  */
+  if (allow_unversioned && !attr)
+    return true;
   gcc_assert (attr);
   attr = TREE_VALUE (TREE_VALUE (attr));
   return (TREE_CODE (attr) == STRING_CST
diff --git a/gcc/attribs.h b/gcc/attribs.h
index 00a83a785b4..a87680b702b 100644
--- a/gcc/attribs.h
+++ b/gcc/attribs.h
@@ -55,8 +55,10 @@  extern struct scoped_attributes *
 
 extern char *sorted_attr_string (tree);
 extern bool common_function_versions (tree, tree);
-extern tree make_dispatcher_decl (const tree);
-extern bool is_function_default_version (const tree);
+extern tree
+make_dispatcher_decl (const tree, const char *name = NULL);
+extern bool
+is_function_default_version (const tree, bool = false);
 extern void handle_ignored_attributes_option (vec<char *> *);
 
 /* Return a type like TTYPE except that its TYPE_ATTRIBUTES
diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
index 9dbee9b6fae..8856422cd22 100644
--- a/gcc/cgraphunit.cc
+++ b/gcc/cgraphunit.cc
@@ -635,6 +635,7 @@  cgraph_node::analyze (void)
   input_location = DECL_SOURCE_LOCATION (decl);
   semantic_interposition = opt_for_fn (decl, flag_semantic_interposition);
 
+  cgraph_function_version_info *version_info = function_version ();
   if (thunk)
     {
       thunk_info *info = thunk_info::get (this);
@@ -661,12 +662,27 @@  cgraph_node::analyze (void)
     resolve_alias (cgraph_node::get (alias_target), transparent_alias);
   else if (dispatcher_function)
     {
-      /* Generate the dispatcher body of multi-versioned functions.  */
-      cgraph_function_version_info *dispatcher_version_info
-	= function_version ();
-      if (dispatcher_version_info != NULL
-          && (dispatcher_version_info->dispatcher_resolver
-	      == NULL_TREE))
+      /* Check for the default version.  */
+      if (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
+	  && !is_function_default_version (
+	    function_version ()->next->this_node->decl, true)
+	  && callers)
+	{
+	  error_at (callers->call_stmt->location,
+		    "Call to multiversioned function without a default version "
+		    "declaration in scope.");
+	  return;
+	}
+
+      /* Generate the dispatcher body of multi-versioned functions at
+	 the first point where the dispatched symbol has been called.  */
+      if ((!TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
+	   || (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL && version_info
+	       && version_info->next
+	       && is_function_default_version (
+		 version_info->next->this_node->decl, true)
+	       && version_info->next->this_node->definition))
+	  && (version_info && version_info->dispatcher_resolver == NULL_TREE))
 	{
 	  tree resolver = NULL_TREE;
 	  gcc_assert (targetm.generate_version_dispatcher_body);
@@ -703,6 +719,14 @@  cgraph_node::analyze (void)
 
       pop_cfun ();
     }
+
+  /* We need to always process the dispatcher resolver in the
+     TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL fmv case.  */
+  if (version_info && TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
+      && !dispatcher_function && is_function_default_version (decl, true))
+    enqueue_node (cgraph_node::get_create (
+      targetm.get_function_versions_dispatcher (decl)));
+
   analyzed = true;
 
   input_location = saved_loc;
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index f0672420aed..172c49107b7 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -20364,6 +20364,18 @@  aarch64_mangle_decl_assembler_name (tree decl, tree id)
   return id;
 }
 
+static std::string
+get_assembler_name_without_default (tree default_decl)
+{
+  std::string name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (default_decl));
+
+  auto size = name.size ();
+  if (size >= 8 && name.compare (size - 8, 8, ".default") == 0)
+    name.resize (size - 8);
+
+  return name;
+}
+
 /* Return an identifier for the base assembler name of a versioned function.
    This is computed by taking the default version's assembler name, and
    stripping off the ".default" suffix if it's already been appended.  */
@@ -20371,11 +20383,7 @@  aarch64_mangle_decl_assembler_name (tree decl, tree id)
 static tree
 get_suffixed_assembler_name (tree default_decl, const char *suffix)
 {
-  std::string name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (default_decl));
-
-  auto size = name.size ();
-  if (size >= 8 && name.compare (size - 8, 8, ".default") == 0)
-    name.resize (size - 8);
+  std::string name = get_assembler_name_without_default (default_decl);
   name += suffix;
   return get_identifier (name.c_str());
 }
@@ -20839,8 +20847,19 @@  aarch64_get_function_versions_dispatcher (void *decl)
       struct cgraph_node *dispatcher_node = NULL;
       struct cgraph_function_version_info *dispatcher_version_info = NULL;
 
+      /* Strip the suffix from the default version of the function to get the
+	 dispatcher name. */
+
+      std::string name
+	= get_assembler_name_without_default (default_node->decl);
+
+      dispatch_decl = make_dispatcher_decl (default_node->decl, name.c_str ());
+
+      /* Mark the assembler name as set to prevent it getting mangled again .*/
+      SET_DECL_ASSEMBLER_NAME (dispatch_decl,
+			       DECL_ASSEMBLER_NAME (dispatch_decl));
+
       /* Right now, the dispatching is done via ifunc.  */
-      dispatch_decl = make_dispatcher_decl (default_node->decl);
       TREE_NOTHROW (dispatch_decl) = TREE_NOTHROW (fn);
 
       dispatcher_node = cgraph_node::get_create (dispatch_decl);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index f463f1061e9..f45bef7d849 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -1412,6 +1412,8 @@  extern enum aarch64_code_model aarch64_cmodel;
 
 #define TARGET_HAS_FMV_TARGET_ATTRIBUTE 0
 
+#define TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL 1
+
 #define TARGET_SUPPORTS_WIDE_INT 1
 
 /* Modes valid for AdvSIMD D registers, i.e. that fit in half a Q register.  */
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index c11b91d75ce..f98177b180e 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -1403,7 +1403,7 @@  add_method (tree type, tree method, bool via_using)
       /* If these are versions of the same function, process and
 	 move on.  */
       if (TREE_CODE (fn) == FUNCTION_DECL
-	  && maybe_version_functions (method, fn, true))
+	  && maybe_version_functions (method, fn))
 	continue;
 
       if (DECL_INHERITED_CTOR (method))
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b3c909b05c4..724f0eb9032 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6998,7 +6998,7 @@  extern void determine_local_discriminator	(tree, tree = NULL_TREE);
 extern bool member_like_constrained_friend_p	(tree);
 extern bool fns_correspond			(tree, tree);
 extern int decls_match				(tree, tree, bool = true);
-extern bool maybe_version_functions		(tree, tree, bool);
+extern bool maybe_version_functions (tree, tree);
 extern bool validate_constexpr_redeclaration	(tree, tree);
 extern bool merge_default_template_args		(tree, tree, bool);
 extern tree duplicate_decls			(tree, tree,
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index b4e7ceefedb..244400d0893 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -1213,9 +1213,7 @@  decls_match (tree newdecl, tree olddecl, bool record_versions /* = true */)
 	  && targetm.target_option.function_versions (newdecl, olddecl))
 	{
 	  if (record_versions)
-	    maybe_version_functions (newdecl, olddecl,
-				     (!DECL_FUNCTION_VERSIONED (newdecl)
-				      || !DECL_FUNCTION_VERSIONED (olddecl)));
+	    maybe_version_functions (newdecl, olddecl);
 	  return 0;
 	}
     }
@@ -1286,7 +1284,7 @@  maybe_mark_function_versioned (tree decl)
    If RECORD is set to true, record function versions.  */
 
 bool
-maybe_version_functions (tree newdecl, tree olddecl, bool record)
+maybe_version_functions (tree newdecl, tree olddecl)
 {
   if (!targetm.target_option.function_versions (newdecl, olddecl))
     return false;
@@ -1309,8 +1307,7 @@  maybe_version_functions (tree newdecl, tree olddecl, bool record)
       maybe_mark_function_versioned (newdecl);
     }
 
-  if (record)
-    cgraph_node::record_function_versions (olddecl, newdecl);
+  cgraph_node::record_function_versions (olddecl, newdecl);
 
   return true;
 }
@@ -18409,6 +18406,15 @@  start_preparsed_function (tree decl1, tree attrs, int flags)
   if (!DECL_OMP_DECLARE_REDUCTION_P (decl1))
     start_lambda_scope (decl1);
 
+  /* To enable versions to be created across TU's we mark and mangle all
+     non-default versioned functions.  */
+  if (TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
+      && lookup_attribute (TARGET_HAS_FMV_TARGET_ATTRIBUTE ? "target"
+							   : "target_version",
+			   DECL_ATTRIBUTES (decl1)))
+    if (!is_function_default_version (decl1, true))
+      maybe_mark_function_versioned (decl1);
+
   return true;
 }
 
diff --git a/gcc/defaults.h b/gcc/defaults.h
index 918e3ec2f24..de0095ef42a 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -879,6 +879,16 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define TARGET_CLONES_ATTR_SEPARATOR ','
 #endif
 
+/* Indicates if the target should generate FMV dispatchers at the
+   site of the default version implementation rather than at the call sites
+   to the function.
+   The creation at default dispatcher is as defined in the Arm C language
+   extension and esures batter behavior when defining function versions
+   accross translation units.  */
+#ifndef TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL
+#define TARGET_CREATE_FMV_DISPATCHER_AT_DEFAULT_IMPL 0
+#endif
+
 /* Select a format to encode pointers in exception handling data.  We
    prefer those that result in fewer dynamic relocations.  Assume no
    special support here and encode direct references.  */
diff --git a/gcc/testsuite/g++.target/aarch64/mv-1.C b/gcc/testsuite/g++.target/aarch64/mv-1.C
index b4b0e5e3fea..5798e41d155 100644
--- a/gcc/testsuite/g++.target/aarch64/mv-1.C
+++ b/gcc/testsuite/g++.target/aarch64/mv-1.C
@@ -36,3 +36,7 @@  int bar()
 /* { dg-final { scan-assembler-times "\n_Z3foov\._Mrng:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._MrngMflagm:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._Mflagm:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\tbl\t_Z3foov\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 1 } } */
diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols2.C b/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
index f0c7967a97a..7c8e1bcbed1 100644
--- a/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
+++ b/gcc/testsuite/g++.target/aarch64/mv-symbols2.C
@@ -40,13 +40,13 @@  int foo (int)
 /* { dg-final { scan-assembler-times "\n_Z3foov\.default:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._Mdotprod:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._MsveMsve2:\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 0 } } */
+/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 1 } } */
 
 /* { dg-final { scan-assembler-times "\n_Z3fooi\.default:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3fooi\._Mdotprod:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3fooi\._MsveMsve2:\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n_Z3fooi\.resolver:\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3fooi, %gnu_indirect_function\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3fooi,_Z3fooi\.resolver\n" 0 } } */
+/* { dg-final { scan-assembler-times "\n_Z3fooi\.resolver:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3fooi, %gnu_indirect_function\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3fooi,_Z3fooi\.resolver\n" 1 } } */
diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols3.C b/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
index 3d30e27deb8..2e8a27f0522 100644
--- a/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
+++ b/gcc/testsuite/g++.target/aarch64/mv-symbols3.C
@@ -28,10 +28,10 @@  int bar()
 /* { dg-final { scan-assembler-times "\n_Z3foov\.default:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._Mdotprod:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._MsveMsve2:\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n\tbl\t_Z3foov\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 0 } } */
+/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 0 } } */
 
 /* { dg-final { scan-assembler-times "\n_Z3fooi\.default:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n_Z3fooi\._Mdotprod:\n" 0 } } */
diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols4.C b/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
index 73e3279ec31..04cac9d118a 100644
--- a/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
+++ b/gcc/testsuite/g++.target/aarch64/mv-symbols4.C
@@ -43,6 +43,6 @@  int bar()
 /* { dg-final { scan-assembler-times "\n_Z3fooi\.default:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3fooi\._Mdotprod:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n_Z3fooi\._MsveMsve2:\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n_Z3fooi\.resolver:\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3fooi, %gnu_indirect_function\n" 0 } } */
-/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3fooi,_Z3fooi\.resolver\n" 0 } } */
+/* { dg-final { scan-assembler-times "\n_Z3fooi\.resolver:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3fooi, %gnu_indirect_function\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3fooi,_Z3fooi\.resolver\n" 1 } } */
diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols5.C b/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
index 05d1379f53e..faa67909c64 100644
--- a/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
+++ b/gcc/testsuite/g++.target/aarch64/mv-symbols5.C
@@ -43,10 +43,10 @@  int bar()
 /* { dg-final { scan-assembler-times "\n_Z3foov\.default:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._Mdotprod:\n" 1 } } */
 /* { dg-final { scan-assembler-times "\n_Z3foov\._MsveMsve2:\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n\tbl\t_Z3foov\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 1 } } */
-/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n\t\.type\t_Z3foov, %gnu_indirect_function\n" 0 } } */
+/* { dg-final { scan-assembler-times "\n\t\.set\t_Z3foov,_Z3foov\.resolver\n" 0 } } */
 
 /* { dg-final { scan-assembler-times "\n_Z3fooi\.default:\n" 0 } } */
 /* { dg-final { scan-assembler-times "\n_Z3fooi\._Mdotprod:\n" 1 } } */
diff --git a/gcc/testsuite/g++.target/aarch64/mv-symbols6.C b/gcc/testsuite/g++.target/aarch64/mv-symbols6.C
new file mode 100644
index 00000000000..70d21c832ab
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/mv-symbols6.C
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O0" } */
+
+__attribute__ ((target_version ("default"))) int
+foo ()
+{
+  return 1;
+}
+
+int bar()
+{
+  return foo();
+}
+
+/* It is not overly clear what the correct behaviour is in this case.
+   This test serves more as a test of consistency for this case rather
+   than a test of correctness.  */
+
+/* { dg-final { scan-assembler-times "\n_Z3foov:\n" 1 } } */
+/* { dg-final { scan-assembler-times "\n_Z3foov\.default:\n" 0 } } */
+/* { dg-final { scan-assembler-times "\n_Z3foov\.resolver:\n" 0 } } */
+/* { dg-final { scan-assembler-times "bl\t_Z3foov\n" 1 } } */