[PATCH] Multiversioning fixes (PR c++/55742)

Jakub Jelinek jakub@redhat.com
Fri Jan 18 20:22:00 GMT 2013


Hi!

As discussed in the PR, this patch attempts to change multiversioning
so that backwards compatibility with the previous target attribute behavior
is preserved.  In particular, e.g. for
void foo () __attribute__((target ("avx")));
void foo ()
{
}

the foo definition was just merged with the previous declaration and
used the avx target attribute.  The patch introduces
target ("default")
which should be used if a default multiversion variant is desirable.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I've mentioned a bunch of other issues with multiversioning in the PR
(including missing documentation for the feature), hope those can be
resolved incrementally.

2013-01-18  Jakub Jelinek  <jakub@redhat.com>

	PR c++/55742
	* config/i386/i386.c (ix86_valid_target_attribute_inner_p): Diagnose
	invalid args instead of ICEing on it.
	(ix86_valid_target_attribute_p): Allow target("default") attribute.
	(sorted_attr_string): Formatting fix.
	(ix86_mangle_function_version_assembler_n): Mangle target("default")
	as if no target attribute is present.
	(ix86_function_versions): Don't return true if one of the decls
	doesn't have target attribute.  If they don't and one of the decls
	is DECL_FUNCTION_VERSIONED, report an error.
	(is_function_default_version): Return true if there is
	target("default") attribute rather than no target attribute at all.

	* g++.dg/mv1.C: Adjust test.
	* g++.dg/mv2.C: Likewise.
	* g++.dg/mv5.C: Likewise.
	* g++.dg/mv6.C: Likewise.

--- gcc/config/i386/i386.c.jj	2013-01-18 17:59:41.692943971 +0100
+++ gcc/config/i386/i386.c	2013-01-18 18:43:21.105326012 +0100
@@ -4223,7 +4223,10 @@ ix86_valid_target_attribute_inner_p (tre
     }
 
   else if (TREE_CODE (args) != STRING_CST)
-    gcc_unreachable ();
+    {
+      error ("invalid %<target%> attribute value");
+      return false;
+    }
 
   /* Handle multiple arguments separated by commas.  */
   next_optstr = ASTRDUP (TREE_STRING_POINTER (args));
@@ -4433,6 +4436,15 @@ ix86_valid_target_attribute_p (tree fnde
 {
   struct cl_target_option cur_target;
   bool ret = true;
+
+  /* attribute((target("default"))) does nothing, beyond
+     affecting multi-versioning.  */
+  if (TREE_VALUE (args)
+      && TREE_CODE (TREE_VALUE (args)) == STRING_CST
+      && TREE_CHAIN (args) == NULL_TREE
+      && strcmp (TREE_STRING_POINTER (TREE_VALUE (args)), "default") == 0)
+    return true;
+
   tree old_optimize = build_optimization_node ();
   tree new_target, new_optimize;
   tree func_optimize = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl);
@@ -28964,7 +28976,7 @@ sorted_attr_string (const char *str)
     if (str[i] == ',')
       argnum++;
 
-  attr_str = (char *)xmalloc (strlen (str) + 1);
+  attr_str = (char *) xmalloc (strlen (str) + 1);
   strcpy (attr_str, str);
 
   /* Replace "=,-" with "_".  */
@@ -29034,6 +29046,9 @@ ix86_mangle_function_version_assembler_n
   version_string
     = TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (version_attr)));
 
+  if (strcmp (version_string, "default") == 0)
+    return id;
+
   attr_str = sorted_attr_string (version_string);
   assembler_name = (char *) xmalloc (strlen (orig_name)
 				     + strlen (attr_str) + 2);
@@ -29058,7 +29073,7 @@ ix86_function_versions (tree fn1, tree f
   const char *attr_str1, *attr_str2;
   char *target1, *target2;
   bool result;
-  
+
   if (TREE_CODE (fn1) != FUNCTION_DECL
       || TREE_CODE (fn2) != FUNCTION_DECL)
     return false;
@@ -29070,12 +29085,35 @@ ix86_function_versions (tree fn1, tree f
   if (attr1 == NULL_TREE && attr2 == NULL_TREE)
     return false;
 
-  /* If one function does not have a target attribute, these are versions.  */
+  /* Diagnose missing target attribute if one of the decls is already
+     multi-versioned.  */
   if (attr1 == NULL_TREE || attr2 == NULL_TREE)
-    return true;
+    {
+      if (DECL_FUNCTION_VERSIONED (fn1) || DECL_FUNCTION_VERSIONED (fn2))
+	{
+	  if (attr2 != NULL_TREE)
+	    {
+	      tree tem = fn1;
+	      fn1 = fn2;
+	      fn2 = tem;
+	      attr1 = attr2;
+	    }
+	  error_at (DECL_SOURCE_LOCATION (fn2),
+		    "missing %<target%> attribute for multi-versioned %D",
+		    fn2);
+	  error_at (DECL_SOURCE_LOCATION (fn1),
+		    "previous declaration of %D", fn1);
+	  /* Prevent diagnosing of the same error multiple times.  */
+	  DECL_ATTRIBUTES (fn2)
+	    = tree_cons (get_identifier ("target"),
+			 copy_node (TREE_VALUE (attr1)),
+			 DECL_ATTRIBUTES (fn2));
+	}
+      return false;
+    }
 
-  attr_str1 =  TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr1)));
-  attr_str2 =  TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr2)));
+  attr_str1 = TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr1)));
+  attr_str2 = TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr2)));
 
   target1 = sorted_attr_string (attr_str1);
   target2 = sorted_attr_string (attr_str2);
@@ -29196,9 +29234,14 @@ make_dispatcher_decl (const tree decl)
 static bool
 is_function_default_version (const tree decl)
 {
-  return (TREE_CODE (decl) == FUNCTION_DECL
-	  && DECL_FUNCTION_VERSIONED (decl)
-	  && lookup_attribute ("target", DECL_ATTRIBUTES (decl)) == NULL_TREE);
+  if (TREE_CODE (decl) != FUNCTION_DECL
+      || !DECL_FUNCTION_VERSIONED (decl))
+    return false;
+  tree attr = lookup_attribute ("target", DECL_ATTRIBUTES (decl));
+  gcc_assert (attr);
+  attr = TREE_VALUE (TREE_VALUE (attr));
+  return (TREE_CODE (attr) == STRING_CST
+	  && strcmp (TREE_STRING_POINTER (attr), "default") == 0);
 }
 
 /* Make a dispatcher declaration for the multi-versioned function DECL.
--- gcc/testsuite/g++.dg/mv1.C.jj	2013-01-18 17:59:41.567944606 +0100
+++ gcc/testsuite/g++.dg/mv1.C	2013-01-18 18:31:36.223533556 +0100
@@ -6,7 +6,7 @@
 #include <assert.h>
 
 /* Default version.  */
-int foo ();
+int foo () __attribute__ ((target("default")));
 /* The other versions of foo.  Mix up the ordering and 
    check if the dispatching does it in the order of priority. */
 /* Check combination of target attributes.  */
@@ -65,7 +65,8 @@ int main ()
   return 0;
 }
 
-int foo ()
+int __attribute__ ((target("default")))
+foo ()
 {
   return 0;
 }
--- gcc/testsuite/g++.dg/mv2.C.jj	2013-01-18 17:59:41.456945279 +0100
+++ gcc/testsuite/g++.dg/mv2.C	2013-01-18 18:31:36.224533535 +0100
@@ -7,7 +7,7 @@
 #include <assert.h>
 
 /* Default version.  */
-int foo ();
+int foo () __attribute__ ((target ("default")));
 /* The dispatch checks should be in the exact reverse order of the
    declarations below.  */
 int foo () __attribute__ ((target ("mmx")));
@@ -51,7 +51,7 @@ int main ()
   return 0;
 }
 
-int
+int __attribute__ ((target("default")))
 foo ()
 {
   return 0;
--- gcc/testsuite/g++.dg/mv5.C.jj	2013-01-18 17:59:41.587944512 +0100
+++ gcc/testsuite/g++.dg/mv5.C	2013-01-18 18:31:36.224533535 +0100
@@ -7,7 +7,7 @@
 
 
 /* Default version.  */
-inline int
+inline int __attribute__ ((target ("default")))
 foo ()
 {
   return 0;
--- gcc/testsuite/g++.dg/mv6.C.jj	2013-01-18 17:59:41.507944953 +0100
+++ gcc/testsuite/g++.dg/mv6.C	2013-01-18 18:31:36.224533535 +0100
@@ -8,6 +8,7 @@ class Foo
 {
  public:
   /* Default version of foo.  */
+  __attribute__ ((target("default")))
   int foo ()
   {
     return 0;


	Jakub



More information about the Gcc-patches mailing list