From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12604 invoked by alias); 5 Oct 2012 18:32:29 -0000 Received: (qmail 12596 invoked by uid 22791); 5 Oct 2012 18:32:28 -0000 X-SWARE-Spam-Status: No, hits=-8.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 05 Oct 2012 18:32:20 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q95IWHqg002134 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 5 Oct 2012 14:32:17 -0400 Received: from [10.3.113.34] (ovpn-113-34.phx2.redhat.com [10.3.113.34]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q95IWG9Z013472; Fri, 5 Oct 2012 14:32:16 -0400 Message-ID: <506F27AF.3070805@redhat.com> Date: Fri, 05 Oct 2012 18:32:00 -0000 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 MIME-Version: 1.0 To: Sriraman Tallam CC: Xinliang David Li , mark@codesourcery.com, nathan@codesourcery.com, "H.J. Lu" , Richard Guenther , Jan Hubicka , Uros Bizjak , reply@codereview.appspotmail.com, gcc-patches@gcc.gnu.org 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> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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/msg00548.txt.bz2 On 08/24/2012 08:34 PM, Sriraman Tallam wrote: > + /* For function versions, their parms and types match > + but they are not duplicates. Record function versions > + as and when they are found. */ > + if (TREE_CODE (fn) == FUNCTION_DECL > + && TREE_CODE (method) == FUNCTION_DECL > + && (DECL_FUNCTION_SPECIFIC_TARGET (fn) > + || DECL_FUNCTION_SPECIFIC_TARGET (method)) > + && targetm.target_option.function_versions (fn, method)) > + { > + targetm.set_version_assembler_name (fn); > + targetm.set_version_assembler_name (method); > + continue; > + } This seems like an odd place to be setting assembler names; better to just have the existing mangle_decl_assembler_name hook add the appropriate suffix when it's called normally. > + Also, mark this function as needed if it is marked inline but > + is a multi-versioned function. */ Why? If it's used, it should be marked needed though the normal process. > + error_at (location_of (DECL_NAME (OVL_CURRENT (fn))), > + "Call to multiversioned function %<%D(%A)%> with" > + " no default version", DECL_NAME (OVL_CURRENT (fn)), > + build_tree_list_vec (*args)); location_of just returns input_location if you ask it for the location of an identifier, so you might as well use error with no explicit location. And why not print candidates->fn instead of pasting the name/args? Also, lowercase "call". > + { > + tree dispatcher_decl = NULL; > + struct cgraph_node *node = cgraph_get_node (fn); > + if (node != NULL) > + dispatcher_decl = cgraph_get_node (fn)->version_dispatcher_decl; > + if (dispatcher_decl == NULL) > + { > + error_at (input_location, "Call to multiversioned function" > + " without a default is not allowed"); > + return NULL; > + } > + retrofit_lang_decl (dispatcher_decl); > + gcc_assert (dispatcher_decl != NULL); > + fn = dispatcher_decl; > + } Let's move this logic into a separate function that returns the dispatcher function. > + /* Both functions must be marked versioned. */ > + gcc_assert (DECL_FUNCTION_VERSIONED (cand1->fn) > + && DECL_FUNCTION_VERSIONED (cand2->fn)); Why can't you compare a versioned function and a non-versioned one? The code in joust should go further down in the function, before the handling of two declarations of the same function. > + /* For multiversioned functions, aggregate all the versions here for > + generating the dispatcher body later if necessary. */ > + > + if (TREE_CODE (candidates->fn) == FUNCTION_DECL > + && DECL_FUNCTION_VERSIONED (candidates->fn)) > + { > + VEC (tree, heap) *fn_ver_vec = NULL; > + struct z_candidate *ver = candidates; > + fn_ver_vec = VEC_alloc (tree, heap, 2); > + for (;ver; ver = ver->next) > + VEC_safe_push (tree, heap, fn_ver_vec, ver->fn); > + gcc_assert (targetm.get_function_versions_dispatcher); > + targetm.get_function_versions_dispatcher (fn_ver_vec); > + VEC_free (tree, heap, fn_ver_vec); > + } This seems to assume that all the functions in the list of candidates are versioned, but there might be unrelated functions from different namespaces. Also, doing this every time someone calls a versioned function seems like the wrong place; I would think it would be better to build up a list of versions as you seed declarations, and then use that list to define the dispatcher at EOF if it's needed. > + if (TREE_CODE (decl) == FUNCTION_DECL > + && DECL_FUNCTION_VERSIONED (decl) > + && DECL_ASSEMBLER_NAME_SET_P (decl)) > + write_source_name (DECL_ASSEMBLER_NAME (decl)); > + else > + write_source_name (DECL_NAME (decl)); Again, I think it's better to handle the suffix via mangle_decl_assembler_name. Jason