diff mbox

User directed Function Multiversioning via Function Overloading (issue5752064)

Message ID CAAs8Hmw-WXwbgUvwmegd680i0rT8CG2M=XYRa1V5ejMqbRd1Bw@mail.gmail.com
State New
Headers show

Commit Message

Sriraman Tallam Nov. 10, 2012, 1:33 a.m. UTC 
Hi Jason,

   Made all the changes and attached patch. Ok to commit?

Thanks,
-Sri.

On Tue, Nov 6, 2012 at 7:52 AM, Jason Merrill <jason@redhat.com> wrote:
> On 11/05/2012 09:38 PM, Sriraman Tallam wrote:
>>
>> +      /* For multi-versioned functions, more than one match is just fine.
>>
>> +        Call decls_match to make sure they are different because they are
>> +        versioned.  */
>> +      if (DECL_FUNCTION_VERSIONED (fn))
>> +       {
>> +          for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN
>> (match))
>> +           if (!DECL_FUNCTION_VERSIONED (TREE_PURPOSE (match))
>> +               || decls_match (fn, TREE_PURPOSE (match)))
>> +             break;
>> +       }
>
>
> I still don't understand what this code is supposed to be doing.  Please
> remove it and instead modify the other loop to allow mismatches that are
> versions of the same function.
>
>> +  /* If the olddecl is a version, so is the newdecl.  */
>> +  if (TREE_CODE (newdecl) == FUNCTION_DECL
>> +      && DECL_FUNCTION_VERSIONED (olddecl))
>> +    {
>> +      DECL_FUNCTION_VERSIONED (newdecl) = 1;
>> +      /* newdecl will be purged and is no longer a version.  */
>> +      delete_function_version (newdecl);
>> +    }
>
>
> Please make the comment clearer that the reason we're setting the flag on
> the newdecl is so that it'll be copied back into the olddecl; otherwise it
> seems odd to say it's a version and then it isn't a version.
>
>> +  /* If a pointer to a function that is multi-versioned is requested, the
>> +     pointer to the dispatcher function is returned instead.  This works
>> +     well because indirectly calling the function will dispatch the right
>> +     function version at run-time.  */
>>
>> +  if (DECL_FUNCTION_VERSIONED (fn))
>> +    {
>> +      tree dispatcher_decl = NULL;
>> +      gcc_assert (targetm.get_function_versions_dispatcher);
>> +      dispatcher_decl = targetm.get_function_versions_dispatcher (fn);
>> +      if (!dispatcher_decl)
>> +       {
>> +         error_at (input_location, "Pointer to a multiversioned function"
>> +                   " without a default is not allowed");
>> +         return error_mark_node;
>> +       }
>> +      retrofit_lang_decl (dispatcher_decl);
>> +      fn = dispatcher_decl;
>
>
> This code should use the get_function_version_dispatcher function in
> cp/call.c.
>
> Jason
>
* cgraph.c (insert_new_cgraph_node_version): Use cgraph_get_node
	instead of cgraph_get_create_node.
	* cp/class.c (mark_versions_used): Remove.
	(resolve_address_of_overloaded_function): Do not call decls_match
	for versioned functions. Call get_function_versions_dispatcher.
	* cp/decl.c (duplicate_decls): Add comments.
	* cp/call.c (get_function_version_dispatcher): Expose function.
	(mark_versions_used): Expose function.
	* cp/cp-tree.h (mark_versions_used): New declaration.
	(get_function_version_dispatcher): Ditto.
	* testsuite/g++.dg/mv4.C: Add require ifunc. Change error message.
	* testsuite/g++.dg/mv5.C: Add require ifunc.
	* testsuite/g++.dg/mv6.C: Add require ifunc.

Comments

Jason Merrill Nov. 12, 2012, 5:04 a.m. UTC | #1 
On 11/09/2012 08:33 PM, Sriraman Tallam wrote:
> +	  /* Skip calling decls_match for versioned functions.  */
> +          if (DECL_FUNCTION_VERSIONED (fn)
> +	      && DECL_FUNCTION_VERSIONED (TREE_PURPOSE (match)))
> +	    continue;
> +	  if (!decls_match (fn, TREE_PURPOSE (match)))
> +            break;

This seems like it would allow multiple versioned functions from 
different namespaces; I want to allow mismatches only if they are 
versions of the same function.  I was thinking

          for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN 
(match))
   	    if (!decls_match (fn, TREE_PURPOSE (match))
                 && !targetm.target_option.function_versions (fn, 
TREE_PURPOSE (match)))
	      break;

> +      error_at (input_location, "Call/Pointer to multiversioned function"
> +                " without a default cannot be dispatched");

Let's just say "use of multiversioned  function without a default".

Jason
diff mbox

Patch

Index: cgraph.c
===================================================================
--- cgraph.c	(revision 193385)
+++ cgraph.c	(working copy)
@@ -206,7 +206,7 @@  insert_new_cgraph_node_version (struct cgraph_node
 void
 delete_function_version (tree decl)
 {
-  struct cgraph_node *decl_node = cgraph_get_create_node (decl);
+  struct cgraph_node *decl_node = cgraph_get_node (decl);
   struct cgraph_function_version_info *decl_v = NULL;
 
   if (decl_node == NULL)
Index: testsuite/g++.dg/mv4.C
===================================================================
--- testsuite/g++.dg/mv4.C	(revision 193385)
+++ testsuite/g++.dg/mv4.C	(working copy)
@@ -3,6 +3,7 @@ 
    and its pointer is taken.  */
 
 /* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" }  */
 /* { dg-options "-O2 -mno-sse -mno-popcnt" } */
 
 int __attribute__ ((target ("sse")))
@@ -18,6 +19,6 @@  foo ()
 
 int main ()
 {
-  int (*p)() = &foo; /* { dg-error "Pointer to a multiversioned function without a default is not allowed" {} } */
+  int (*p)() = &foo; /* { dg-error "Pointer/Call to multiversioned function without a default is not allowed" {} } */
   return (*p)();
 }
Index: testsuite/g++.dg/mv6.C
===================================================================
--- testsuite/g++.dg/mv6.C	(revision 193385)
+++ testsuite/g++.dg/mv6.C	(working copy)
@@ -1,6 +1,7 @@ 
 /* Test to check if member version multiversioning works correctly.  */
 
 /* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" }  */
 
 class Foo
 {
Index: testsuite/g++.dg/mv5.C
===================================================================
--- testsuite/g++.dg/mv5.C	(revision 193385)
+++ testsuite/g++.dg/mv5.C	(working copy)
@@ -2,6 +2,7 @@ 
    marked comdat with inline keyword.  */
 
 /* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" }  */
 /* { dg-options "-O2  -mno-popcnt" } */
 
 
Index: cp/class.c
===================================================================
--- cp/class.c	(revision 193385)
+++ cp/class.c	(working copy)
@@ -7068,38 +7068,6 @@  pop_lang_context (void)
 {
   current_lang_name = VEC_pop (tree, current_lang_base);
 }
-
-/* fn is a function version dispatcher that is marked used. Mark all the 
-   semantically identical function versions it will dispatch as used.  */
-
-static void
-mark_versions_used (tree fn)
-{
-  struct cgraph_node *node;
-  struct cgraph_function_version_info *node_v;
-  struct cgraph_function_version_info *it_v;
-
-  gcc_assert (TREE_CODE (fn) == FUNCTION_DECL);
-
-  node = cgraph_get_node (fn);
-  if (node == NULL)
-    return;
-
-  gcc_assert (node->dispatcher_function);
-
-  node_v = get_cgraph_node_version (node);
-  if (node_v == NULL)
-    return;
-
-  /* All semantically identical versions are chained.  Traverse and mark each
-     one of them as used.  */
-  it_v = node_v->next;
-  while (it_v != NULL)
-    {
-      mark_used (it_v->this_node->symbol.decl);
-      it_v = it_v->next;
-    }
-}
 
 /* Type instantiation routines.  */
 
@@ -7315,22 +7283,17 @@  resolve_address_of_overloaded_function (tree targe
 
       fn = TREE_PURPOSE (matches);
 
-      /* For multi-versioned functions, more than one match is just fine.
-	 Call decls_match to make sure they are different because they are
-	 versioned.  */
-      if (DECL_FUNCTION_VERSIONED (fn))
+      /* For multi-versioned functions, more than one match is just fine and
+	 decls_match will return false as they are different.  */
+      for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN (match))
 	{
-          for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN (match))
-  	    if (!DECL_FUNCTION_VERSIONED (TREE_PURPOSE (match))
-	        || decls_match (fn, TREE_PURPOSE (match)))
-	      break;
+	  /* Skip calling decls_match for versioned functions.  */
+          if (DECL_FUNCTION_VERSIONED (fn)
+	      && DECL_FUNCTION_VERSIONED (TREE_PURPOSE (match)))
+	    continue;
+	  if (!decls_match (fn, TREE_PURPOSE (match)))
+            break;
 	}
-      else
-	{
-          for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN (match))
-  	    if (!decls_match (fn, TREE_PURPOSE (match)))
-	      break;
-	}
 
       if (match)
 	{
@@ -7377,17 +7340,9 @@  resolve_address_of_overloaded_function (tree targe
      function version at run-time.  */
   if (DECL_FUNCTION_VERSIONED (fn))
     {
-      tree dispatcher_decl = NULL;
-      gcc_assert (targetm.get_function_versions_dispatcher);
-      dispatcher_decl = targetm.get_function_versions_dispatcher (fn);
-      if (!dispatcher_decl)
-	{
-	  error_at (input_location, "Pointer to a multiversioned function"
-		    " without a default is not allowed");
-	  return error_mark_node;
-	}
-      retrofit_lang_decl (dispatcher_decl);
-      fn = dispatcher_decl;
+      fn = get_function_version_dispatcher (fn);
+      if (fn == NULL)
+	return error_mark_node;
       /* Mark all the versions corresponding to the dispatcher as used.  */
       if (!(flags & tf_conv))
 	mark_versions_used (fn);
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 193385)
+++ cp/decl.c	(working copy)
@@ -2307,12 +2307,15 @@  duplicate_decls (tree newdecl, tree olddecl, bool
   else if (DECL_PRESERVE_P (newdecl))
     DECL_PRESERVE_P (olddecl) = 1;
 
-  /* If the olddecl is a version, so is the newdecl.  */
+  /* Merge the DECL_FUNCTION_VERSIONED information.  newdecl will be copied
+     to olddecl and deleted.  */
   if (TREE_CODE (newdecl) == FUNCTION_DECL
       && DECL_FUNCTION_VERSIONED (olddecl))
     {
+      /* Set the flag for newdecl so that it gets copied to olddecl.  */
       DECL_FUNCTION_VERSIONED (newdecl) = 1;
-      /* newdecl will be purged and is no longer a version.  */
+      /* newdecl will be purged after copying to olddecl and is no longer
+         a version.  */
       delete_function_version (newdecl);
     }
 
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 193385)
+++ cp/call.c	(working copy)
@@ -6517,7 +6517,7 @@  magic_varargs_p (tree fn)
 
 /* Returns the decl of the dispatcher function if FN is a function version.  */
 
-static tree
+tree
 get_function_version_dispatcher (tree fn)
 {
   tree dispatcher_decl = NULL;
@@ -6530,8 +6530,8 @@  get_function_version_dispatcher (tree fn)
 
   if (dispatcher_decl == NULL)
     {
-      error_at (input_location, "Call to multiversioned function"
-                " without a default is not allowed");
+      error_at (input_location, "Call/Pointer to multiversioned function"
+                " without a default cannot be dispatched");
       return NULL;
     }
 
@@ -6543,7 +6543,7 @@  get_function_version_dispatcher (tree fn)
 /* fn is a function version dispatcher that is marked used. Mark all the 
    semantically identical function versions it will dispatch as used.  */
 
-static void
+void
 mark_versions_used (tree fn)
 {
   struct cgraph_node *node;
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 193385)
+++ cp/cp-tree.h	(working copy)
@@ -4971,6 +4971,8 @@  extern bool is_list_ctor			(tree);
 #ifdef ENABLE_CHECKING
 extern void validate_conversion_obstack		(void);
 #endif /* ENABLE_CHECKING */
+extern void mark_versions_used			(tree);
+extern tree get_function_version_dispatcher	(tree);
 
 /* in class.c */
 extern tree build_vfield_ref			(tree, tree);