Patchwork Multiversioning fixes (PR c++/55742, take 2)

login
register
mail settings
Submitter Sriraman Tallam
Date Feb. 8, 2013, 12:10 a.m.
Message ID <CAAs8Hmwfj-A0Jn6z8sU2tQeq4hU=k_GwjtYWoSA00xE=fmeptQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/219023/
State New
Headers show

Comments

Sriraman Tallam - Feb. 8, 2013, 12:10 a.m.
On Thu, Feb 7, 2013 at 6:29 AM, Jason Merrill <jason@redhat.com> wrote:
> On 02/06/2013 08:39 PM, Sriraman Tallam wrote:
>>
>> +// Test to check if an error is generated when virtual functions
>> +// are multiversioned.
>
>
> This seems like a TODO, rather than something to test for.  I don't see any
> reason why we couldn't support multiversioned virtual functions.
>
>>      error_at (DECL_SOURCE_LOCATION (decl),
>> -             "Virtual function versioning not supported\n");
>> +             "Virtual function multiversioning not supported\n");
>
>
> And so this should be a sorry rather than an error.

Attached a new patch with these changes. Also made more minor changes
to config/i386/i386.c.

Thanks
Sri

>
> Jason
>
* doc/extend.texi: Document Function Multiversioning and "default"
	parameter string to target attribute.
	* g++.dg/ext/mv12.C: New test.
	* g++.dg/ext/mv12.h: New file.
	* g++.dg/ext/mv12-aux.C: New file.
	* g++.dg/ext/mv13.C: New test.
	* config/i386/i386.c (get_builtin_code_for_version): Return 0 if
	target attribute parameter is "default".
	(ix86_compare_version_priority): Remove checks for target attribute.
	(ix86_mangle_function_version_assembler_name): Change error to sorry.
	Remove check for target attribute equal to NULL. Add assert.
	(ix86_generate_version_dispatcher_body): Change error to sorry.
Sriraman Tallam - Feb. 11, 2013, 6:35 p.m.
Hi,

  Is this alright to commit?

Thanks
Sri

On Thu, Feb 7, 2013 at 4:10 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Thu, Feb 7, 2013 at 6:29 AM, Jason Merrill <jason@redhat.com> wrote:
>> On 02/06/2013 08:39 PM, Sriraman Tallam wrote:
>>>
>>> +// Test to check if an error is generated when virtual functions
>>> +// are multiversioned.
>>
>>
>> This seems like a TODO, rather than something to test for.  I don't see any
>> reason why we couldn't support multiversioned virtual functions.
>>
>>>      error_at (DECL_SOURCE_LOCATION (decl),
>>> -             "Virtual function versioning not supported\n");
>>> +             "Virtual function multiversioning not supported\n");
>>
>>
>> And so this should be a sorry rather than an error.
>
> Attached a new patch with these changes. Also made more minor changes
> to config/i386/i386.c.
>
> Thanks
> Sri
>
>>
>> Jason
>>
Jason Merrill - Feb. 12, 2013, 3:01 a.m.
OK, thanks.

Jason
Andreas Schwab - Feb. 13, 2013, 9:21 a.m.
Sriraman Tallam <tmsriram@google.com> writes:

> Index: gcc/testsuite/g++.dg/ext/mv12-aux.C
> ===================================================================
> --- gcc/testsuite/g++.dg/ext/mv12-aux.C	(revision 0)
> +++ gcc/testsuite/g++.dg/ext/mv12-aux.C	(revision 0)
> @@ -0,0 +1,11 @@
> +// Test case to check if multiversioning works as expected when the versions
> +// are defined in different files. Auxiliary file for mv12.C.
> +// { dg-do compile }
> +
> +#include "mv12.h"
> +
> +__attribute__ ((target ("sse4.2")))
> +int foo ()

FAIL: g++.dg/ext/mv12-aux.C -std=c++11 (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20130213/gcc/testsuite/g++.dg/ext/mv12.h:5:47: warning: target attribute is not supported on this machine [-Wattributes]
/daten/aranym/gcc/gcc-20130213/gcc/testsuite/g++.dg/ext/mv12.h:6:46: warning: target attribute is not supported on this machine [-Wattributes]
/daten/aranym/gcc/gcc-20130213/gcc/testsuite/g++.dg/ext/mv12-aux.C:8:10: warning: target attribute is not supported on this machine [-Wattributes]

Andreas.

Patch

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 195818)
+++ gcc/doc/extend.texi	(working copy)
@@ -3655,6 +3655,11 @@  Enable/disable the generation of the advanced bit
 @cindex @code{target("aes")} attribute
 Enable/disable the generation of the AES instructions.
 
+@item default
+@cindex @code{target("default")} attribute
+@xref{Function Multiversioning}, where it is used to specify the
+default function version.
+
 @item mmx
 @itemx no-mmx
 @cindex @code{target("mmx")} attribute
@@ -15215,6 +15220,7 @@  Predefined Macros,cpp,The GNU C Preprocessor}).
 * Bound member functions:: You can extract a function pointer to the
                         method denoted by a @samp{->*} or @samp{.*} expression.
 * C++ Attributes::      Variable, function, and type attributes for C++ only.
+* Function Multiversioning::   Declaring multiple function versions.
 * Namespace Association:: Strong using-directives for namespace association.
 * Type Traits::         Compiler support for type traits
 * Java Exceptions::     Tweaking exception handling to work with Java.
@@ -15744,6 +15750,64 @@  interface table mechanism, instead of regular virt
 
 See also @ref{Namespace Association}.
 
+@node Function Multiversioning
+@section Function Multiversioning
+@cindex function versions
+
+With the GNU C++ front end, for target i386, you may specify multiple
+versions of a function, where each function is specialized for a
+specific target feature.  At runtime, the appropriate version of the
+function is automatically executed depending on the characteristics of
+the execution platform.  Here is an example.
+
+@smallexample
+__attribute__ ((target ("default")))
+int foo ()
+@{
+  // The default version of foo.
+  return 0;
+@}
+
+__attribute__ ((target ("sse4.2")))
+int foo ()
+@{
+  // foo version for SSE4.2
+  return 1;
+@}
+
+__attribute__ ((target ("arch=atom")))
+int foo ()
+@{
+  // foo version for the Intel ATOM processor
+  return 2;
+@}
+
+__attribute__ ((target ("arch=amdfam10")))
+int foo ()
+@{
+  // foo version for the AMD Family 0x10 processors.
+  return 3;
+@}
+
+int main ()
+@{
+  int (*p)() = &foo;
+  assert ((*p) () == foo ());
+  return 0;
+@}
+@end smallexample
+
+In the above example, four versions of function foo are created. The
+first version of foo with the target attribute "default" is the default
+version.  This version gets executed when no other target specific
+version qualifies for execution on a particular platform. A new version
+of foo is created by using the same function signature but with a
+different target string.  Function foo is called or a pointer to it is
+taken just like a regular function.  GCC takes care of doing the
+dispatching to call the right version at runtime.  Refer to the
+@uref{http://gcc.gnu.org/wiki/FunctionMultiVersioning, GCC wiki on
+Function Multiversioning} for more details.
+
 @node Namespace Association
 @section Namespace Association
 
Index: gcc/testsuite/g++.dg/ext/mv12-aux.C
===================================================================
--- gcc/testsuite/g++.dg/ext/mv12-aux.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/mv12-aux.C	(revision 0)
@@ -0,0 +1,11 @@ 
+// Test case to check if multiversioning works as expected when the versions
+// are defined in different files. Auxiliary file for mv12.C.
+// { dg-do compile }
+
+#include "mv12.h"
+
+__attribute__ ((target ("sse4.2")))
+int foo ()
+{
+  return 1;
+}
Index: gcc/testsuite/g++.dg/ext/mv12.C
===================================================================
--- gcc/testsuite/g++.dg/ext/mv12.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/mv12.C	(revision 0)
@@ -0,0 +1,22 @@ 
+// Test case to check if multiversioning works as expected when the versions
+// are defined in different files.
+
+// { dg-do run { target i?86-*-* x86_64-*-* } }
+// { dg-require-ifunc "" }
+// { dg-options "-O2" }
+// { dg-additional-sources "mv12-aux.C" }
+
+#include "mv12.h"
+
+int main ()
+{
+  if (__builtin_cpu_supports ("sse4.2"))
+    return foo () - 1;
+  return foo ();
+}
+
+__attribute__ ((target ("default")))
+int foo ()
+{
+  return 0;
+}
Index: gcc/testsuite/g++.dg/ext/mv12.h
===================================================================
--- gcc/testsuite/g++.dg/ext/mv12.h	(revision 0)
+++ gcc/testsuite/g++.dg/ext/mv12.h	(revision 0)
@@ -0,0 +1,6 @@ 
+// Header file used by mv12.C and mv12-aux.C.
+// { dg-do compile { target i?86-*-* x86_64-*-* } }
+// { dg-options "" }
+
+int foo () __attribute__ ((target ("default")));
+int foo () __attribute__ ((target ("sse4.2")));
Index: gcc/testsuite/g++.dg/ext/mv13.C
===================================================================
--- gcc/testsuite/g++.dg/ext/mv13.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/mv13.C	(revision 0)
@@ -0,0 +1,18 @@ 
+// Test case to check if multiversioning functions that are extern "C"
+// generates errors.
+
+// { dg-do compile { target i?86-*-* x86_64-*-* } }
+
+extern "C"
+__attribute__ ((target ("default")))
+int foo ()  // { dg-error "previously defined here" }
+{
+  return 0;
+}
+
+extern "C"
+__attribute__ ((target ("sse4.2")))
+int foo () // { dg-error "redefinition" }
+{
+  return 1;
+}
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 195818)
+++ gcc/config/i386/i386.c	(working copy)
@@ -28695,6 +28695,9 @@  get_builtin_code_for_version (tree decl, tree *pre
   gcc_assert (TREE_CODE (attrs) == STRING_CST);
   attrs_str = TREE_STRING_POINTER (attrs);
 
+  /* Return priority zero for default function.  */
+  if (strcmp (attrs_str, "default") == 0)
+    return 0;
 
   /* Handle arch= if specified.  For priority, set it to be 1 more than
      the best instruction set the processor can handle.  For instance, if
@@ -28827,15 +28830,9 @@  get_builtin_code_for_version (tree decl, tree *pre
 static int
 ix86_compare_version_priority (tree decl1, tree decl2)
 {
-  unsigned int priority1 = 0;
-  unsigned int priority2 = 0;
+  unsigned int priority1 = get_builtin_code_for_version (decl1, NULL);
+  unsigned int priority2 = get_builtin_code_for_version (decl2, NULL);
 
-  if (lookup_attribute ("target", DECL_ATTRIBUTES (decl1)) != NULL)
-    priority1 = get_builtin_code_for_version (decl1, NULL);
-
-  if (lookup_attribute ("target", DECL_ATTRIBUTES (decl2)) != NULL)
-    priority2 = get_builtin_code_for_version (decl2, NULL);
-
   return (int)priority1 - (int)priority2;
 }
 
@@ -29064,14 +29061,12 @@  ix86_mangle_function_version_assembler_name (tree
 
   if (DECL_VIRTUAL_P (decl)
       || DECL_VINDEX (decl))
-    error_at (DECL_SOURCE_LOCATION (decl),
-	      "Virtual function versioning not supported\n");
+    sorry ("Virtual function multiversioning not supported");
 
   version_attr = lookup_attribute ("target", DECL_ATTRIBUTES (decl));
 
-  /* target attribute string is NULL for default functions.  */
-  if (version_attr == NULL_TREE)
-    return id;
+  /* target attribute string cannot be NULL.  */
+  gcc_assert (version_attr != NULL_TREE);
 
   orig_name = IDENTIFIER_POINTER (id);
   version_string
@@ -29511,8 +29506,8 @@  ix86_generate_version_dispatcher_body (void *node_
 	 virtual methods in base classes but are not explicitly marked as
 	 virtual.  */
       if (DECL_VINDEX (versn->symbol.decl))
-        error_at (DECL_SOURCE_LOCATION (versn->symbol.decl),
-		  "Virtual function multiversioning not supported");
+	sorry ("Virtual function multiversioning not supported");
+
       fn_ver_vec.safe_push (versn->symbol.decl);
     }