From patchwork Fri Nov 16 19:55:23 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sriraman Tallam X-Patchwork-Id: 199729 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 081F32C007B for ; Sat, 17 Nov 2012 06:55:38 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1353700539; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=ycNhAmnCjFubNKDQ9CW52+NLYhQ=; b=IEQL/qNyo4Y5soW Q/ut9/ALcSbCtmqFUUiCs+Eb2EkVDgYv4hLGSZhzdMKrlswoOo8rJlOoxx7Tbd8M 3vQXoc2zDHaHjQmxgq7K4Tm4olfsdrESoO9gAtZjmosfrJ9TaSi6ywOXv0SUuxYz Nb4GgRyepNJD1sNM4rwfMUgOHRkM= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:X-Google-DKIM-Signature:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Content-Type:X-Gm-Message-State:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=hyDvowcWj4PRvwTzQd0UGtSh5/3S333ppoZ1ArOmCDO+CLTUZaIkqrkq8rMDLr 0F4c5+Hn9kA+q+OqYocj0Adl+ylKb8LIM1ud+Nfq8LYU0urVZ4fS6orOOXMJ+3vJ AULruDCEWN2IO4PR8KkpVl8fY3Pf8KcakP96KkZobOxRc=; Received: (qmail 4534 invoked by alias); 16 Nov 2012 19:55:33 -0000 Received: (qmail 4523 invoked by uid 22791); 16 Nov 2012 19:55:32 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD, TW_AV X-Spam-Check-By: sourceware.org Received: from mail-ob0-f175.google.com (HELO mail-ob0-f175.google.com) (209.85.214.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 16 Nov 2012 19:55:24 +0000 Received: by mail-ob0-f175.google.com with SMTP id vb8so3213424obc.20 for ; Fri, 16 Nov 2012 11:55:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:x-gm-message-state; bh=eCjdaB7li+TqbpRExEVHQZmoi5KWk+iv4aaYZEXugWM=; b=Rar4vIVLVxfXfNsoRzGDw4zrfZB2MaDlGFs/MVSZRN7tYFLItj3Wt/lLpW6Yr74eju HtN+kd4K8YzWRyfHDvwQBaHIEFlsA/6cSL1+UGBxxaoU97/6R7CB5EYL1SZEs33JAZ7L fUGFmcbbArcp4qJdfg7/FKyaWX5C1OGdzEsn8vfxRzhHkKm/jbS0NTR/9jFAmEeKhQdW dzCy23HkwO84XOsRmHWSSaexaW72UZZzSuoxUMhCsFFWFoJeq8/KU0iKVeibl07aN76l tDAlKvqcEeEduiJ0TDK512/6uPlqSksbSnfWXr5FXl+P8G/EkqzsbxKI7MjMoJOggO2G HXFw== MIME-Version: 1.0 Received: by 10.60.154.231 with SMTP id vr7mr4722269oeb.119.1353095723805; Fri, 16 Nov 2012 11:55:23 -0800 (PST) Received: by 10.182.176.106 with HTTP; Fri, 16 Nov 2012 11:55:23 -0800 (PST) In-Reply-To: References: Date: Fri, 16 Nov 2012 11:55:23 -0800 Message-ID: Subject: Re: [PATCH] Function Multiversioning Bug, checking for function versions From: Sriraman Tallam To: GCC Patches , "H.J. Lu" , "Zamyatin, Igor" , David Li X-Gm-Message-State: ALoCoQmm3XpZqJ7w+MvVjq1SnujE0t2rkcDL8Ct0cbTcYZ0Eohm68pS4NXB02OsJP/y9RpaWi4A+PREYOZRJJSuH8F0c85ki3AW4LFjhUP5uXYet52Ka0sQQRfbWxsh7SzB8SjXzBaRUFNpLv8jVu9kCMuU+pYZpqZ3vhxqZxrGKM2y4iPWJqlMPEnCGwrWG2nMqhHjYiaVE X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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 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 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