Hi,
The previous patch was incomplete because the front-end strips off
invalid target attributes which I did not consider. The attached
updated patch handles this with updated test cases.
Thanks,
-Sri.
On Thu, Nov 15, 2012 at 2:08 PM, Sriraman Tallam <tmsri...@google.com> wrote:
> Hi,
>
> Currently, function multiversioning determines that two functions
> are different by comparing the arch type and isa flags that are set
> after the target string is processed. This leads to cases where the
> versions become identical when the command-line target options are
> altered.
>
> For example, these two versions:
>
> __attribute__ target (("sse4.2")))
> int foo ()
> {
> }
>
> __attribute__ target (("popcnt")))
> int foo ()
> {
> }
>
> become identical when -mpopcnt and -msse4.2 are used while building,
> leading to build errors.
>
> To avoid this, I have modified the function version determination to
> just compare the target string.
> Patch attached. Is this alright to submit?
>
> Thanks,
> -Sri.
* doc/tm.texi.in (TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS): Document
new target hook.
* doc/tm.texi: Regenerate.
* c-family/c-common.c (handle_target_attribute): Retain target attribute
for targets that support versioning.
* target.def (supports_function_versions): New hook.
* config/i386/i386.c (ix86_function_versions): Use target string
to check for function versions instead of target flags.
* testsuite/g++.dg/mv1.C: Remove target options.
* testsuite/g++.dg/mv2.C: Ditto.
* testsuite/g++.dg/mv3.C: Ditto.
* testsuite/g++.dg/mv4.C: Ditto.
* testsuite/g++.dg/mv5.C: Ditto.
* cp/class.c (add_method): Remove calls
to DECL_FUNCTION_SPECIFIC_TARGET.
* config/i386/i386.c (ix86_function_versions): Use target string
to check for function versions instead of target flags.
* (ix86_supports_function_versions): New function.
* (is_function_default_version): Check target string.
* TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS: New macro.
Index: doc/tm.texi
===================================================================
--- doc/tm.texi (revision 193485)
+++ doc/tm.texi (working copy)
@@ -9937,6 +9937,11 @@ different target specific attributes, that is, the
different target machines.
@end deftypefn
+@deftypefn {Target Hook} bool TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS (void)
+This target hook returns @code{true} if the target supports function
+multiversioning.
+@end deftypefn
+
@deftypefn {Target Hook} bool TARGET_CAN_INLINE_P (tree @var{caller}, tree
@var{callee})
This target hook returns @code{false} if the @var{caller} function
cannot inline @var{callee}, based on target specific information. By
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in (revision 193485)
+++ doc/tm.texi.in (working copy)
@@ -9798,6 +9798,11 @@ different target specific attributes, that is, the
different target machines.
@end deftypefn
+@hook TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS
+This target hook returns @code{true} if the target supports function
+multiversioning.
+@end deftypefn
+
@hook TARGET_CAN_INLINE_P
This target hook returns @code{false} if the @var{caller} function
cannot inline @var{callee}, based on target specific information. By
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c (revision 193485)
+++ c-family/c-common.c (working copy)
@@ -8720,8 +8720,12 @@ handle_target_attribute (tree *node, tree name, tr
warning (OPT_Wattributes, "%qE attribute ignored", name);
*no_add_attrs = true;
}
+ /* Do not strip invalid target attributes for targets which support function
+ multiversioning as the target string is used to determine versioned
+ functions. */
else if (! targetm.target_option.valid_attribute_p (*node, name, args,
- flags))
+ flags)
+ && ! targetm.target_option.supports_function_versions ())
*no_add_attrs = true;
return NULL_TREE;
Index: target.def
===================================================================
--- target.def (revision 193485)
+++ target.def (working copy)
@@ -2826,6 +2826,14 @@ DEFHOOK
bool, (tree decl1, tree decl2),
hook_bool_tree_tree_false)
+/* This function returns true if the target supports function
+ multiversioning. */
+DEFHOOK
+(supports_function_versions,
+ "",
+ bool, (void),
+ hool_bool_void_false)
+
/* Function to determine if one function can inline another function. */
#undef HOOK_PREFIX
#define HOOK_PREFIX "TARGET_"
Index: testsuite/g++.dg/mv2.C
===================================================================
--- testsuite/g++.dg/mv2.C (revision 193485)
+++ testsuite/g++.dg/mv2.C (working copy)
@@ -2,7 +2,7 @@
dispatching order when versions are for various ISAs. */
/* { dg-do run { target i?86-*-* x86_64-*-* } } */
/* { dg-require-ifunc "" } */
-/* { dg-options "-O2 -mno-sse -mno-mmx -mno-popcnt -mno-avx" } */
+/* { dg-options "-O2" } */
#include <assert.h>
Index: testsuite/g++.dg/mv4.C
===================================================================
--- testsuite/g++.dg/mv4.C (revision 193486)
+++ testsuite/g++.dg/mv4.C (working copy)
@@ -4,7 +4,7 @@
/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
/* { dg-require-ifunc "" } */
-/* { dg-options "-O2 -mno-sse -mno-popcnt" } */
+/* { dg-options "-O2" } */
int __attribute__ ((target ("sse")))
foo ()
Index: testsuite/g++.dg/mv1.C
===================================================================
--- testsuite/g++.dg/mv1.C (revision 193485)
+++ testsuite/g++.dg/mv1.C (working copy)
@@ -1,7 +1,7 @@
/* Test case to check if Multiversioning works. */
/* { dg-do run { target i?86-*-* x86_64-*-* } } */
/* { dg-require-ifunc "" } */
-/* { dg-options "-O2 -fPIC -mno-avx -mno-popcnt" } */
+/* { dg-options "-O2 -fPIC" } */
#include <assert.h>
Index: testsuite/g++.dg/mv3.C
===================================================================
--- testsuite/g++.dg/mv3.C (revision 193485)
+++ testsuite/g++.dg/mv3.C (working copy)
@@ -10,7 +10,7 @@
test should pass. */
/* { dg-do run { target i?86-*-* x86_64-*-* } } */
-/* { dg-options "-O2 -mno-sse -mno-popcnt" } */
+/* { dg-options "-O2" } */
int __attribute__ ((target ("sse")))
Index: testsuite/g++.dg/mv5.C
===================================================================
--- testsuite/g++.dg/mv5.C (revision 193486)
+++ testsuite/g++.dg/mv5.C (working copy)
@@ -3,7 +3,7 @@
/* { dg-do run { target i?86-*-* x86_64-*-* } } */
/* { dg-require-ifunc "" } */
-/* { dg-options "-O2 -mno-popcnt" } */
+/* { dg-options "-O2" } */
/* Default version. */
Index: cp/class.c
===================================================================
--- cp/class.c (revision 193486)
+++ cp/class.c (working copy)
@@ -1095,8 +1095,6 @@ add_method (tree type, tree method, tree using_dec
&& TREE_CODE (method) == FUNCTION_DECL
&& !DECL_EXTERN_C_P (fn)
&& !DECL_EXTERN_C_P (method)
- && (DECL_FUNCTION_SPECIFIC_TARGET (fn)
- || DECL_FUNCTION_SPECIFIC_TARGET (method))
&& targetm.target_option.function_versions (fn, method))
{
/* Mark functions as versions if necessary. Modify the mangled
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 193486)
+++ config/i386/i386.c (working copy)
@@ -28646,47 +28646,6 @@ dispatch_function_versions (tree dispatch_decl,
return 0;
}
-/* This function returns true if FN1 and FN2 are versions of the same function,
- that is, the targets of the function decls are different. This assumes
- that FN1 and FN2 have the same signature. */
-
-static bool
-ix86_function_versions (tree fn1, tree fn2)
-{
- tree attr1, attr2;
- struct cl_target_option *target1, *target2;
-
- if (TREE_CODE (fn1) != FUNCTION_DECL
- || TREE_CODE (fn2) != FUNCTION_DECL)
- return false;
-
- attr1 = DECL_FUNCTION_SPECIFIC_TARGET (fn1);
- attr2 = DECL_FUNCTION_SPECIFIC_TARGET (fn2);
-
- /* Atleast one function decl should have target attribute specified. */
- if (attr1 == NULL_TREE && attr2 == NULL_TREE)
- return false;
-
- if (attr1 == NULL_TREE)
- attr1 = target_option_default_node;
- else if (attr2 == NULL_TREE)
- attr2 = target_option_default_node;
-
- target1 = TREE_TARGET_OPTION (attr1);
- target2 = TREE_TARGET_OPTION (attr2);
-
- /* target1 and target2 must be different in some way. */
- if (target1->x_ix86_isa_flags == target2->x_ix86_isa_flags
- && target1->x_target_flags == target2->x_target_flags
- && target1->arch == target2->arch
- && target1->tune == target2->tune
- && target1->x_ix86_fpmath == target2->x_ix86_fpmath
- && target1->branch_cost == target2->branch_cost)
- return false;
-
- return true;
-}
-
/* Comparator function to be used in qsort routine to sort attribute
specification strings to "target". */
@@ -28799,6 +28758,60 @@ ix86_mangle_function_version_assembler_name (tree
return get_identifier (assembler_name);
}
+/* This function returns true if FN1 and FN2 are versions of the same function,
+ that is, the target strings of the function decls are different. This
assumes
+ that FN1 and FN2 have the same signature. */
+
+static bool
+ix86_function_versions (tree fn1, tree fn2)
+{
+ tree attr1, attr2;
+ const char *attr_str1, *attr_str2;
+ char *target1, *target2;
+ bool result;
+
+ if (TREE_CODE (fn1) != FUNCTION_DECL
+ || TREE_CODE (fn2) != FUNCTION_DECL)
+ return false;
+
+ attr1 = lookup_attribute ("target", DECL_ATTRIBUTES (fn1));
+ attr2 = lookup_attribute ("target", DECL_ATTRIBUTES (fn2));
+
+ /* Atleast one function decl should have the target attribute specified. */
+ if (attr1 == NULL_TREE && attr2 == NULL_TREE)
+ return false;
+
+ /* If one function does not have a target attribute, these are versions. */
+ if (attr1 == NULL_TREE || attr2 == NULL_TREE)
+ return true;
+
+ 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);
+
+ /* The sorted target strings must be different for fn1 and fn2
+ to be versions. */
+ if (strcmp (target1, target2) == 0)
+ result = false;
+ else
+ result = true;
+
+ free (target1);
+ free (target2);
+
+ return result;
+}
+
+/* This target supports function multiversioning. */
+
+static bool
+ix86_supports_function_versions (void)
+{
+ return true;
+}
+
static tree
ix86_mangle_decl_assembler_name (tree decl, tree id)
{
@@ -28893,7 +28906,7 @@ is_function_default_version (const tree decl)
{
return (TREE_CODE (decl) == FUNCTION_DECL
&& DECL_FUNCTION_VERSIONED (decl)
- && DECL_FUNCTION_SPECIFIC_TARGET (decl) == NULL_TREE);
+ && lookup_attribute ("target", DECL_ATTRIBUTES (decl)) == NULL_TREE);
}
/* Make a dispatcher declaration for the multi-versioned function DECL.
@@ -42154,6 +42167,10 @@ ix86_memmodel_check (unsigned HOST_WIDE_INT val)
#undef TARGET_OPTION_FUNCTION_VERSIONS
#define TARGET_OPTION_FUNCTION_VERSIONS ix86_function_versions
+#undef TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS
+#define TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS \
+ ix86_supports_function_versions
+
#undef TARGET_CAN_INLINE_P
#define TARGET_CAN_INLINE_P ix86_can_inline_p
