From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15902 invoked by alias); 19 Oct 2012 15:10:39 -0000 Received: (qmail 15886 invoked by uid 22791); 19 Oct 2012 15:10:36 -0000 X-SWARE-Spam-Status: No, hits=-6.0 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_CP,TW_MX X-Spam-Check-By: sourceware.org Received: from mail-ie0-f175.google.com (HELO mail-ie0-f175.google.com) (209.85.223.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 19 Oct 2012 15:10:31 +0000 Received: by mail-ie0-f175.google.com with SMTP id c13so810758ieb.20 for ; Fri, 19 Oct 2012 08:10:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=message-id:date:from:organization:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding:x-gm-message-state; bh=4Y7X2KJw31Hq5uOdyhLo5EGvpycy0/+slJ7Jmwy/T/s=; b=HxpsZTJId02D3Qfe+PKqzvcgvfdzmapTdFwWwfburvwh51uOxa9wB8NtDq4afm18+T qSwhloXw2L3VGcvA968GYtx+2wmsH8X+f4qb2xQaAdm1ZKWCrXIbzJC7kAYnneOVzPT3 GY8INn/WXrlX98wxQ1R2YRNYs2PIMNoLPom6RQhrqSyGcCS/NycY51mzR/ah8a5kgxBM tRzEbcyBPRvUC2qvWOKVUMTri5SUeZnUwCUEDWCFG5mt0qpYZlzN+QnVL6MVEBSwwoIO FDJCuUPfwxmb+br6K244/2s0CeOIJ6pHmBY6tpv9byKKQ2y9DEFFV5knf5GcGK6vRyKt dwwg== Received: by 10.50.152.198 with SMTP id va6mr1669531igb.42.1350659430414; Fri, 19 Oct 2012 08:10:30 -0700 (PDT) Received: from dhcp-172-30-221-26.tor.corp.google.com (dhcp-172-30-221-26.tor.corp.google.com [172.30.221.26]) by mx.google.com with ESMTPS id yf6sm1598623igb.0.2012.10.19.08.10.28 (version=SSLv3 cipher=OTHER); Fri, 19 Oct 2012 08:10:28 -0700 (PDT) Message-ID: <50816D63.3020908@google.com> Date: Fri, 19 Oct 2012 15:23:00 -0000 From: Diego Novillo User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:16.0) Gecko/20121010 Thunderbird/16.0.1 MIME-Version: 1.0 To: Sriraman Tallam CC: Jason Merrill , Jan Hubicka , Xinliang David Li , mark@codesourcery.com, nathan@codesourcery.com, "H.J. Lu" , Richard Guenther , Uros Bizjak , reply@codereview.appspotmail.com, GCC Patches Subject: Re: User directed Function Multiversioning via Function Overloading (issue5752064) References: <20120307004630.A503DB21B6@azwildcat.mtv.corp.google.com> <4FF7D1C6.90407@redhat.com> <4FF96D0C.5060406@redhat.com> <4FFBF9F5.6020306@redhat.com> <5008708E.1030109@redhat.com> <506F27AF.3070805@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Gm-Message-State: ALoCoQnMARH8kZAoSWgSdHz3UrTzuKuCe+FdGX7DNgoSilMgAiN6Cz2LqRodt+qK4Wg+qzK7CN+NKuZKajzh9ONw9gArisd33QAzsMFp2Kj9chYDoOLE/1x5laQRsBhW/MAxFEBL3xmH5En8jCvcJDtiDArcM4m8s/y2D983bGMi0AMS11jMqMmmUxC5JHW5DiJXz2R32mCE X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-10/txt/msg01806.txt.bz2 On 2012-10-12 18:19 , Sriraman Tallam wrote: > When the front-end sees more than one decl for "foo", it calls a target hook to > determine if they are versions. To prevent duplicate definition errors with other > versions of "foo", "decls_match" function in cp/decl.c is made to return false > when 2 decls have are deemed versions by the target. This will make all function > versions of "foo" to be added to the overload list of "foo". So, this means that this can only work for C++, right? Or could the same trickery be done some other way in other FEs? I see no handling of different FEs. If the user tries to use these attributes from languages other than C++, we should emit a diagnostic. > +@deftypefn {Target Hook} tree TARGET_GET_FUNCTION_VERSIONS_DISPATCHER (void *@var{arglist}) > +This hook is used to get the dispatcher function for a set of function > +versions. The dispatcher function is called to invoke the rignt function s/rignt/right/ > +version at run-time. @var{arglist} is the vector of function versions > +that should be considered for dispatch. > +@end deftypefn > + > +@deftypefn {Target Hook} tree TARGET_GENERATE_VERSION_DISPATCHER_BODY (void *@var{arg}) > +This hook is used to generate the dispatcher logic to invoke the right > +function version at runtime for a given set of function versions. s/runtime/run-time/ > +@hook TARGET_GET_FUNCTION_VERSIONS_DISPATCHER > +This hook is used to get the dispatcher function for a set of function > +versions. The dispatcher function is called to invoke the rignt function s/rignt/right/ > +version at run-time. @var{arglist} is the vector of function versions > +that should be considered for dispatch. > +@end deftypefn > + > +@hook TARGET_GENERATE_VERSION_DISPATCHER_BODY > +This hook is used to generate the dispatcher logic to invoke the right > +function version at runtime for a given set of function versions. s/runtime/run-time/ > @@ -288,7 +289,6 @@ mark_store (gimple stmt, tree t, void *data) > } > return false; > } > - > /* Create cgraph edges for function calls. > Also look for functions and variables having addresses taken. */ Don't remove vertical white space, please. > + { > + struct cgraph_node *callee = cgraph_get_create_node (decl); > + /* If a call to a multiversioned function dispatcher is > + found, generate the body to dispatch the right function > + at run-time. */ > + if (callee->dispatcher_function) > + { > + tree resolver_decl; > + gcc_assert (callee->function_version.next); What if callee is the last version in the list? Not sure what you are trying to check here. > @@ -8601,9 +8601,22 @@ handle_target_attribute (tree *node, tree name, tr > warning (OPT_Wattributes, "%qE attribute ignored", name); > *no_add_attrs = true; > } > - else if (! targetm.target_option.valid_attribute_p (*node, name, args, > - flags)) > - *no_add_attrs = true; > + else > + { > + /* When a target attribute is invalid, it may also be because the > + target for the compilation unit and the attribute match. For > + instance, target attribute "xxx" is invalid when -mxxx is used. > + When used with multiversioning, removing the attribute will lead > + to duplicate definitions if a default version is provided. > + So, generate a warning here and remove the attribute. */ > + if (!targetm.target_option.valid_attribute_p (*node, name, args, flags)) > + { > + warning (OPT_Wattributes, > + "Invalid target attribute in function %qE, ignored.", > + *node); > + *no_add_attrs = true; If you do this, isn't the compiler going to generate two warning messages? One for the invalid target attribute, the second for the duplicate definition. > @@ -228,6 +228,26 @@ struct GTY(()) cgraph_node { > struct cgraph_node *prev_sibling_clone; > struct cgraph_node *clones; > struct cgraph_node *clone_of; > + > + /* Function Multiversioning info. */ > + struct { > + /* Chains all the semantically identical function versions. The > + first function in this chain is the default function. */ > + struct cgraph_node *prev; > + /* If this node is a dispatcher for function versions, this points > + to the default function version, the first function in the chain. */ > + struct cgraph_node *next; Why not a VEC of function decls? Seems easier to manage and less size overhead. > @@ -3516,8 +3522,8 @@ struct GTY(()) tree_function_decl { > unsigned looping_const_or_pure_flag : 1; > unsigned has_debug_args_flag : 1; > unsigned tm_clone_flag : 1; > - > - /* 1 bit left */ > + unsigned versioned_function : 1; > + /* No bits left. */ You ate the last bit! How rude ;) > @@ -8132,6 +8176,38 @@ joust (struct z_candidate *cand1, struct z_candida > && (IS_TYPE_OR_DECL_P (cand1->fn))) > return 1; > > + /* For Candidates of a multi-versioned function, make the version with s/Candidates/candidates/ > + old_current_function_decl = current_function_decl; > + push_cfun (DECL_STRUCT_FUNCTION (function_decl)); > + current_function_decl = function_decl; push_cfun will set current_function_decl for you. No need to keep track of old_current_function_decl. > + enum feature_priority > + { > + P_ZERO = 0, > + P_MMX, > + P_SSE, > + P_SSE2, > + P_SSE3, > + P_SSSE3, > + P_PROC_SSSE3, > + P_SSE4_a, > + P_PROC_SSE4_a, > + P_SSE4_1, > + P_SSE4_2, > + P_PROC_SSE4_2, > + P_POPCNT, > + P_AVX, > + P_AVX2, > + P_FMA, > + P_PROC_FMA > + }; There's no need to have this list dynamically defined, right? > + } > + } > + > + /* Process feature name. */ > + tok_str = (char *) xmalloc (strlen (attrs_str) + 1); XNEWVEC(char, strlen (attrs_str) + 1); > + /* Atleast one more version other than the default. */ s/Atleast/At least/ > + num_versions = VEC_length (tree, fndecls); > + gcc_assert (num_versions >= 2); > + > + function_version_info = (struct _function_version_info *) > + xmalloc ((num_versions - 1) * sizeof (struct _function_version_info)); Better use VEC() here. > + > + /* The first version in the vector is the default decl. */ > + default_decl = VEC_index (tree, fndecls, 0); > + > + old_current_function_decl = current_function_decl; > + push_cfun (DECL_STRUCT_FUNCTION (dispatch_decl)); > + current_function_decl = dispatch_decl; No need to set current_function_decl. > + > + gseq = bb_seq (*empty_bb); > + /* Function version dispatch is via IFUNC. IFUNC resolvers fire before > + constructors, so explicity call __builtin_cpu_init here. */ > + ifunc_cpu_init_stmt = gimple_build_call_vec ( > + ix86_builtins [(int) IX86_BUILTIN_CPU_INIT], NULL); > + gimple_seq_add_stmt (&gseq, ifunc_cpu_init_stmt); > + gimple_set_bb (ifunc_cpu_init_stmt, *empty_bb); > + set_bb_seq (*empty_bb, gseq); > + > + pop_cfun (); > + current_function_decl = old_current_function_decl; Likewise here. > +/* This function returns true if fn1 and fn2 are versions of the same function. > + Returns false if only one of the function decls has the target attribute > + set or if the targets of the function decls are different. This assumes > + the fn1 and fn2 have the same signature. */ Mention the arguments in capitals. > + for (i = 0; i < strlen (str); i++) > + if (str[i] == ',') > + argnum++; > + > + attr_str = (char *)xmalloc (strlen (str) + 1); XNEWVEC() > + strcpy (attr_str, str); > + > + /* Replace "=,-" with "_". */ > + for (i = 0; i < strlen (attr_str); i++) > + if (attr_str[i] == '=' || attr_str[i]== '-') > + attr_str[i] = '_'; > + > + if (argnum == 1) > + return attr_str; > + > + args = (char **)xmalloc (argnum * sizeof (char *)); VEC()? > + if (DECL_DECLARED_INLINE_P (decl) > + && lookup_attribute ("gnu_inline", > + DECL_ATTRIBUTES (decl))) > + error_at (DECL_SOURCE_LOCATION (decl), > + "Function versions cannot be marked as gnu_inline," > + " bodies have to be generated\n"); No newline at the end of the error message. > + sprintf (assembler_name, "%s.%s", orig_name, attr_str); > + if (dump_file) > + fprintf (stderr, "Assembler name set to %s for function version %s\n", > + assembler_name, IDENTIFIER_POINTER (id)); This dumps to stderr instead of dump_file. Also, use the new dumping facility? > +/* Return a new name by appending SUFFIX to the DECL name. If > + make_unique is true, append the full path name. */ Full path name of what? > + > +static char * > +make_name (tree decl, const char *suffix, bool make_unique) > +{ > + char *global_var_name; > + int name_len; > + const char *name; > + const char *unique_name = NULL; > + > + name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)); > + > + /* Get a unique name that can be used globally without any chances > + of collision at link time. */ > + if (make_unique) > + unique_name = IDENTIFIER_POINTER (get_file_function_name ("\0")); > + > + name_len = strlen (name) + strlen (suffix) + 2; > + > + if (make_unique) > + name_len += strlen (unique_name) + 1; > + global_var_name = (char *) xmalloc (name_len); XNEWVEC. Diego.