On 2012-11-16 14:55 , Sriraman Tallam wrote:
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.
Looks mostly OK.
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))
I don't follow. Given the new hook, why do you need to remove this check?
+/* 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)
+{
Any particular reason why you moved this function down here?
+ 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. */
s/Atleast/At least/
Diego.
