From patchwork Sat Aug 10 13:47:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 266227 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]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 9E6022C00A9 for ; Sat, 10 Aug 2013 23:47:43 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=v4yG6Bup/58YJgqqf tWIILSAbmzRl12l1q/FOYywA4b1bsryVCSeqoxnkhBWw341kq/kk8Bnpivigv/n5 GGetpFiu0DHl8Bwbqq98Ivk04nDdpQptz3TXVScpPubjszrLvj6+5JW/Bzt/1EpM x2dE0YVIB6pvAt5suzlCv4VEKY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=f5WX2y3HDZ/erA1W8AR7yX3 z6Rg=; b=Z+UQEsq4P8MKKbXR1tBj+gOxKJvc6AaKED/STZwt25cEkTSqyk35AM+ uvp9cHqY/sFLODgm7qoHbblVIJsltdOVYFT8G4wwbByyablJ9oXCyFf229xlpPQm GxP2F2ql0NjwFb3Lzt23q9keHXPoP8weVzdh7ZlVajCiSc1qa8VM= Received: (qmail 29784 invoked by alias); 10 Aug 2013 13:47:36 -0000 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 Received: (qmail 29758 invoked by uid 89); 10 Aug 2013 13:47:35 -0000 X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_NO, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RDNS_NONE autolearn=no version=3.3.1 Received: from Unknown (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sat, 10 Aug 2013 13:47:34 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 8CBD2543A96; Sat, 10 Aug 2013 15:47:25 +0200 (CEST) Date: Sat, 10 Aug 2013 15:47:25 +0200 From: Jan Hubicka To: Jan Hubicka Cc: Andi Kleen , gcc-patches@gcc.gnu.org, marxin.liska@gmail.com, mjambor@suse.cz, davidxl@google.com Subject: Re: Speculative call support in the callgraph Message-ID: <20130810134725.GB613@kam.mff.cuni.cz> References: <20130809121840.GA28721@kam.mff.cuni.cz> <87pptmo6n3.fsf@tassilo.jf.intel.com> <20130809231511.GC12350@kam.mff.cuni.cz> <20130810002521.GH19750@two.firstfloor.org> <20130810041914.GI19750@two.firstfloor.org> <20130810093946.GB9171@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130810093946.GB9171@kam.mff.cuni.cz> User-Agent: Mutt/1.5.20 (2009-06-14) > Hi, > also I just became aware that this patch hits the following bug of gold > http://sourceware.org/bugzilla/show_bug.cgi?id=14342 > It makes gcc.dg/tree-prof/crossmodule-indircall-1.c fail with the bogus > diagnostic on TLS and non-TLS use of the same symbol. I attached the > details into the PR. GNU-LD works. > > I am not sure if I can workaround this problem except for convincing users to > always use -fno-lto with -fprofile-generate. Shall we fix it in mainline > binutils + document that earlier binutils are broken with -fporfile-generate > -flto? Hi, it seems that the bug also appeared in later GNU-LDs. So I made the following workaround that with -flto switches back to old API of indirect_call_profiler and it makes new hidden variables in profiling code. This avoid mixing of accesses of the TLS variable from LTO and non-LTO code. Unless someone suggests me a better solution soon, I will go with this hack. It ought to be essentially harmless just adding some extra overhead to -flto -fprofile-generate (same as we had before patch). Things will also still break if you produce fat LTO -fprofile-generate objects and link some with LTO and others without. I do not think we need to support this with binutils bug. Honza Index: libgcc/Makefile.in =================================================================== --- libgcc/Makefile.in (revision 201539) +++ libgcc/Makefile.in (working copy) @@ -857,7 +857,7 @@ LIBGCOV = _gcov _gcov_merge_add _gcov_me _gcov_execv _gcov_execvp _gcov_execve _gcov_reset _gcov_dump \ _gcov_interval_profiler _gcov_pow2_profiler _gcov_one_value_profiler \ _gcov_indirect_call_profiler _gcov_average_profiler _gcov_ior_profiler \ - _gcov_merge_ior + _gcov_merge_ior _gcov_indirect_call_profiler_v2 libgcov-objects = $(patsubst %,%$(objext),$(LIBGCOV)) Index: libgcc/libgcov.c =================================================================== --- libgcc/libgcov.c (revision 201539) +++ libgcc/libgcov.c (working copy) @@ -1120,6 +1120,8 @@ __gcov_one_value_profiler (gcov_type *co #endif #ifdef L_gcov_indirect_call_profiler +/* This function exist only for workaround of binutils bug 14342. + Once this compatibility hack is obsolette, it can be removed. */ /* By default, the C++ compiler will use function addresses in the vtable entries. Setting TARGET_VTABLE_USES_DESCRIPTORS to nonzero @@ -1141,18 +1143,90 @@ __gcov_one_value_profiler (gcov_type *co /* Tries to determine the most common value among its inputs. */ void __gcov_indirect_call_profiler (gcov_type* counter, gcov_type value, - void* cur_func, void* callee_func) + void* cur_func, void* callee_func) { /* If the C++ virtual tables contain function descriptors then one function may have multiple descriptors and we need to dereference the descriptors to see if they point to the same function. */ if (cur_func == callee_func || (VTABLE_USES_DESCRIPTORS && callee_func - && *(void **) cur_func == *(void **) callee_func)) + && *(void **) cur_func == *(void **) callee_func)) __gcov_one_value_profiler_body (counter, value); } + +#endif +#ifdef L_gcov_indirect_call_profiler_v2 + +/* These two variables are used to actually track caller and callee. Keep + them in TLS memory so races are not common (they are written to often). + The variables are set directly by GCC instrumented code, so declaration + here must match one in tree-profile.c */ + +#ifdef HAVE_CC_TLS +__thread +#endif +void * __gcov_indirect_call_callee; +#ifdef HAVE_CC_TLS +__thread +#endif +gcov_type * __gcov_indirect_call_counters; + +/* By default, the C++ compiler will use function addresses in the + vtable entries. Setting TARGET_VTABLE_USES_DESCRIPTORS to nonzero + tells the compiler to use function descriptors instead. The value + of this macro says how many words wide the descriptor is (normally 2), + but it may be dependent on target flags. Since we do not have access + to the target flags here we just check to see if it is set and use + that to set VTABLE_USES_DESCRIPTORS to 0 or 1. + + It is assumed that the address of a function descriptor may be treated + as a pointer to a function. */ + +#ifdef TARGET_VTABLE_USES_DESCRIPTORS +#define VTABLE_USES_DESCRIPTORS 1 +#else +#define VTABLE_USES_DESCRIPTORS 0 #endif +/* Tries to determine the most common value among its inputs. */ +void +__gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func) +{ + /* If the C++ virtual tables contain function descriptors then one + function may have multiple descriptors and we need to dereference + the descriptors to see if they point to the same function. */ + if (cur_func == __gcov_indirect_call_callee + || (VTABLE_USES_DESCRIPTORS && __gcov_indirect_call_callee + && *(void **) cur_func == *(void **) __gcov_indirect_call_callee)) + __gcov_one_value_profiler_body (__gcov_indirect_call_counters, value); +} +#endif + +#ifdef L_gcov_time_profiler + +static int function_counter = 0; + +void +__gcov_time_profiler (gcov_type* counters) +{ + function_counter++; + + /* counters[0] indicates a first visit of the function. */ + if (counters[0] == 0) + { + counters[0] = function_counter; + fprintf (stderr, "__gcov_time_profiler:%d\n", counters[0]); + } + fprintf (stderr, "__gcov_time_profiler:%d\n", counters[0]); + + /* counters[1] is last visit of the function. */ + counters[1] = function_counter; + + /* counters[2] indicated if visited */ + if (counters[2] == 0) + counters[2] = 1; +} +#endif #ifdef L_gcov_average_profiler /* Increase corresponding COUNTER by VALUE. FIXME: Perhaps we want Index: gcc/tree-profile.c =================================================================== --- gcc/tree-profile.c (revision 201640) +++ gcc/tree-profile.c (working copy) @@ -67,13 +67,28 @@ init_ic_make_global_vars (void) ptr_void = build_pointer_type (void_type_node); - ic_void_ptr_var - = build_decl (UNKNOWN_LOCATION, VAR_DECL, - get_identifier ("__gcov_indirect_call_callee"), - ptr_void); + /* Workaround for binutils bug 14342. Once it is fixed, remove lto path. */ + if (flag_lto) + { + ic_void_ptr_var + = build_decl (UNKNOWN_LOCATION, VAR_DECL, + get_identifier ("__gcov_indirect_call_callee_ltopriv"), + ptr_void); + TREE_PUBLIC (ic_void_ptr_var) = 1; + DECL_COMMON (ic_void_ptr_var) = 1; + DECL_VISIBILITY (ic_void_ptr_var) = VISIBILITY_HIDDEN; + DECL_VISIBILITY_SPECIFIED (ic_void_ptr_var) = true; + } + else + { + ic_void_ptr_var + = build_decl (UNKNOWN_LOCATION, VAR_DECL, + get_identifier ("__gcov_indirect_call_callee"), + ptr_void); + TREE_PUBLIC (ic_void_ptr_var) = 1; + DECL_EXTERNAL (ic_void_ptr_var) = 1; + } TREE_STATIC (ic_void_ptr_var) = 1; - TREE_PUBLIC (ic_void_ptr_var) = 1; - DECL_EXTERNAL (ic_void_ptr_var) = 1; DECL_ARTIFICIAL (ic_void_ptr_var) = 1; DECL_INITIAL (ic_void_ptr_var) = NULL; if (targetm.have_tls) @@ -83,13 +98,28 @@ init_ic_make_global_vars (void) varpool_finalize_decl (ic_void_ptr_var); gcov_type_ptr = build_pointer_type (get_gcov_type ()); - ic_gcov_type_ptr_var - = build_decl (UNKNOWN_LOCATION, VAR_DECL, - get_identifier ("__gcov_indirect_call_counters"), - gcov_type_ptr); + /* Workaround for binutils bug 14342. Once it is fixed, remove lto path. */ + if (flag_lto) + { + ic_gcov_type_ptr_var + = build_decl (UNKNOWN_LOCATION, VAR_DECL, + get_identifier ("__gcov_indirect_call_counters_ltopriv"), + gcov_type_ptr); + TREE_PUBLIC (ic_gcov_type_ptr_var) = 1; + DECL_COMMON (ic_gcov_type_ptr_var) = 1; + DECL_VISIBILITY (ic_gcov_type_ptr_var) = VISIBILITY_HIDDEN; + DECL_VISIBILITY_SPECIFIED (ic_gcov_type_ptr_var) = true; + } + else + { + ic_gcov_type_ptr_var + = build_decl (UNKNOWN_LOCATION, VAR_DECL, + get_identifier ("__gcov_indirect_call_counters"), + gcov_type_ptr); + TREE_PUBLIC (ic_gcov_type_ptr_var) = 1; + DECL_EXTERNAL (ic_gcov_type_ptr_var) = 1; + } TREE_STATIC (ic_gcov_type_ptr_var) = 1; - TREE_PUBLIC (ic_gcov_type_ptr_var) = 1; - DECL_EXTERNAL (ic_gcov_type_ptr_var) = 1; DECL_ARTIFICIAL (ic_gcov_type_ptr_var) = 1; DECL_INITIAL (ic_gcov_type_ptr_var) = NULL; if (targetm.have_tls) @@ -157,15 +187,31 @@ gimple_init_edge_profiler (void) init_ic_make_global_vars (); - /* void (*) (gcov_type, void *) */ - ic_profiler_fn_type - = build_function_type_list (void_type_node, - gcov_type_node, - ptr_void, - NULL_TREE); - tree_indirect_call_profiler_fn - = build_fn_decl ("__gcov_indirect_call_profiler_v2", - ic_profiler_fn_type); + /* Workaround for binutils bug 14342. Once it is fixed, remove lto path. */ + if (flag_lto) + { + /* void (*) (gcov_type, void *) */ + ic_profiler_fn_type + = build_function_type_list (void_type_node, + gcov_type_ptr, gcov_type_node, + ptr_void, ptr_void, + NULL_TREE); + tree_indirect_call_profiler_fn + = build_fn_decl ("__gcov_indirect_call_profiler", + ic_profiler_fn_type); + } + else + { + /* void (*) (gcov_type, void *) */ + ic_profiler_fn_type + = build_function_type_list (void_type_node, + gcov_type_node, + ptr_void, + NULL_TREE); + tree_indirect_call_profiler_fn + = build_fn_decl ("__gcov_indirect_call_profiler_v2", + ic_profiler_fn_type); + } TREE_NOTHROW (tree_indirect_call_profiler_fn) = 1; DECL_ATTRIBUTES (tree_indirect_call_profiler_fn) = tree_cons (get_identifier ("leaf"), NULL, @@ -375,8 +421,25 @@ gimple_gen_ic_func_profiler (void) true, GSI_SAME_STMT); tree_uid = build_int_cst (gcov_type_node, cgraph_get_node (current_function_decl)->profile_id); - stmt1 = gimple_build_call (tree_indirect_call_profiler_fn, 2, - tree_uid, cur_func); + /* Workaround for binutils bug 14342. Once it is fixed, remove lto path. */ + if (flag_lto) + { + tree counter_ptr, ptr_var; + counter_ptr = force_gimple_operand_gsi (&gsi, ic_gcov_type_ptr_var, + true, NULL_TREE, true, + GSI_SAME_STMT); + ptr_var = force_gimple_operand_gsi (&gsi, ic_void_ptr_var, + true, NULL_TREE, true, + GSI_SAME_STMT); + + stmt1 = gimple_build_call (tree_indirect_call_profiler_fn, 4, + counter_ptr, tree_uid, cur_func, ptr_var); + } + else + { + stmt1 = gimple_build_call (tree_indirect_call_profiler_fn, 2, + tree_uid, cur_func); + } gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT); /* Set __gcov_indirect_call_callee to 0,