From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48537 invoked by alias); 6 Sep 2016 12:45:48 -0000 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 Received: (qmail 48508 invoked by uid 89); 6 Sep 2016 12:45:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=Increase, UD:COPYING.RUNTIME, UD:RUNTIME, copyingruntime X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Sep 2016 12:45:37 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id ADFAEAB1A; Tue, 6 Sep 2016 12:45:34 +0000 (UTC) Subject: Re: [PATCH] Detect whether target can use -fprofile-update=atomic To: David Edelsohn , Nathan Sidwell References: <53c4f874443d63de99393a9816041ba75f1d605e.1470041316.git.mliska@suse.cz> <95628fa2-8719-bb82-d1ab-ca8d46355020@acm.org> <0ec1ad3f-0fc7-507e-878c-907adae1c011@suse.cz> <0e248978-3717-1d78-b3b1-9fc60ded5c8e@suse.cz> <882f0f98-9b33-a764-833b-ca61796f3143@acm.org> <87f2bc4f-c4df-eadd-aec6-a937ed0ccaba@acm.org> <1253ac69-3301-f185-e43a-a34cadf8f51e@suse.cz> <67fda6d2-9b3e-a0d1-effc-34e1115030b2@acm.org> <1ff3cc75-7cee-79f3-395b-ef7a4d286a3d@acm.org> <04a05835-4666-4d7d-c1a9-d4bcc4ea924a@suse.cz> <19c4a81d-6ecd-8c6e-b641-e257c1959baf@suse.cz> <43f60306-d6d6-47e4-758c-a806f661d6a0@acm.org> <07cadca7-913a-acad-2f49-482b19e97a72@suse.cz> Cc: Jakub Jelinek , GCC Patches , Jan Hubicka , Andreas Schwab , Richard Biener From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: Date: Tue, 06 Sep 2016 12:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------573245037DB7C61514F91303" X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00300.txt.bz2 This is a multi-part message in MIME format. --------------573245037DB7C61514F91303 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 2344 On 09/06/2016 02:38 PM, David Edelsohn wrote: > On Tue, Sep 6, 2016 at 8:14 AM, Nathan Sidwell wrote: >> On 09/06/16 06:57, David Edelsohn wrote: >> >>> What about Jakub's comment in the PR? >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77378#c6 >> >> >> This needs addressing. Can you clarify PPC behaviour, because I may have >> misunderstood: >> >> 1) PPC currently has 64 bit counters -- but cannot support 64bit atomic ops >> in 32 bit mode. >> >> 2) PPC currently has 32 bit counters anyway. > > The rs6000 port ABIs implement 64 bit "long long" type. The current > code uses LONG_LONG_TYPE_SIZE for the counters. I assume that most > ports don't support native 64 bit atomic operations in 32 bit ABI -- > PowerPC does not. > > The previous code allowed gcov type to be overridden, but I don't > think that it was 32 bit on most targets. > >> >> I had interpreted the comment to be implying #2, but now I'm not so sure. >> >>> The proposed patch seems wrong or at least incomplete. The recent >>> change is imposing a 64 bit DImode counter when producing 32 bit code. >>> PowerPC does support 64 bit atomic operations in 32 bit mode. >> >> >> I'm presuming you've missed a 'NOT' in that sentence. > > Yes, I omitted a "NOT". > > PowerPC64 has 64 bit atomics, but PowerPC32 subset only provides 32 > bit atomics in the ISA. > > If the counters always should be 64 bit, then a poor-man's 64 bit > atomic operation proposed by Jakub seems like a better solution. > > Thanks, David Hi David. I sent the previous email before I read the Jakub's comment. I'm attaching simplified patch (based of the comment), which works for i386 target. I'm testing that on on m68k target. I prefer to have a single type for one config, same what Nathan suggested. I like the idea of poor-man's atomics, I can make an incremental patch. Martin > >> >>> Was there a design decision that profile counters always should be 64 >>> bits? Either 32 bit targets won't support multi-threaded profiling or >>> 32 bit targets can overflow the counter sooner. >> >> >> >>> Which is worse? >>> Which is more likely? >> >> >> My initial thought is that it is probably awkward to support 2 different >> sized counter types in the 'same' config. I.e. 64-bit single-threaded >> counters and 32-bit threaded counters. >> >> nathan --------------573245037DB7C61514F91303 Content-Type: text/x-patch; name="0001-PATCH-Detect-whether-target-can-use-fprofile-update-.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-PATCH-Detect-whether-target-can-use-fprofile-update-.pa"; filename*1="tch" Content-length: 5587 >From 44289abf2e3ecfb7e17c6f204b280af06bf20b0e Mon Sep 17 00:00:00 2001 From: marxin Date: Tue, 6 Sep 2016 14:35:52 +0200 Subject: [PATCH] [PATCH] Detect whether target can use -fprofile-update=atomic libgcc/ChangeLog: 2016-09-06 Martin Liska * libgcov-profiler.c: Use __GCC_HAVE_SYNC_COMPARE_AND_SWAP_{4,8} to conditionaly enable/disable *_atomic functions. gcc/ChangeLog: 2016-09-06 Martin Liska * tree-profile.c (tree_profiling): Detect whether target can use -fprofile-update=atomic. gcc/testsuite/ChangeLog: 2016-09-06 Martin Liska * gcc.dg/profile-update-warning.c: New test. --- gcc/testsuite/gcc.dg/profile-update-warning.c | 7 +++++++ gcc/tree-profile.c | 17 +++++++++++++++++ libgcc/libgcov-profiler.c | 24 +++++++++++++++++++----- 3 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/profile-update-warning.c diff --git a/gcc/testsuite/gcc.dg/profile-update-warning.c b/gcc/testsuite/gcc.dg/profile-update-warning.c new file mode 100644 index 0000000..0614fad --- /dev/null +++ b/gcc/testsuite/gcc.dg/profile-update-warning.c @@ -0,0 +1,7 @@ +/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */ +/* { dg-options "-fprofile-update=atomic -fprofile-generate -march=i386 -m32" } */ + +int main(int argc, char *argv[]) +{ + return 0; +} /* { dg-warning "target does not support atomic profile update, single mode is selected" } */ diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c index 622869e..8ce35be 100644 --- a/gcc/tree-profile.c +++ b/gcc/tree-profile.c @@ -528,6 +528,13 @@ gimple_gen_ior_profiler (histogram_value value, unsigned tag, unsigned base) gsi_insert_before (&gsi, call, GSI_NEW_STMT); } +#ifndef HAVE_sync_compare_and_swapsi +#define HAVE_sync_compare_and_swapsi 0 +#endif +#ifndef HAVE_atomic_compare_and_swapsi +#define HAVE_atomic_compare_and_swapsi 0 +#endif + /* Profile all functions in the callgraph. */ static unsigned int @@ -535,6 +542,16 @@ tree_profiling (void) { struct cgraph_node *node; + /* Verify whether we can utilize atomic update operations. */ + if (flag_profile_update == PROFILE_UPDATE_ATOMIC + && !HAVE_sync_compare_and_swapsi + && !HAVE_atomic_compare_and_swapsi) + { + warning (0, "target does not support atomic profile update, " + "single mode is selected"); + flag_profile_update = PROFILE_UPDATE_SINGLE; + } + /* This is a small-ipa pass that gets called only once, from cgraphunit.c:ipa_passes(). */ gcc_assert (symtab->state == IPA_SSA); diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c index 70a821d..887041f 100644 --- a/libgcc/libgcov-profiler.c +++ b/libgcc/libgcov-profiler.c @@ -24,8 +24,20 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see . */ #include "libgcov.h" +#include "auto-target.h" #if !defined(inhibit_libc) +/* Detect whether target can support atomic update of profilers. */ +#if LONG_LONG_TYPE_SIZE <= 32 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 +#define GCOV_SUPPORTS_ATOMIC 1 +#else +#if LONG_LONG_TYPE_SIZE > 32 && __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 +#define GCOV_SUPPORTS_ATOMIC 1 +#else +#define GCOV_SUPPORTS_ATOMIC 0 +#endif +#endif + #ifdef L_gcov_interval_profiler /* If VALUE is in interval , then increases the corresponding counter in COUNTERS. If the VALUE is above or below @@ -46,7 +58,7 @@ __gcov_interval_profiler (gcov_type *counters, gcov_type value, } #endif -#ifdef L_gcov_interval_profiler_atomic +#if defined(L_gcov_interval_profiler_atomic) && GCOV_SUPPORTS_ATOMIC /* If VALUE is in interval , then increases the corresponding counter in COUNTERS. If the VALUE is above or below the interval, COUNTERS[STEPS] or COUNTERS[STEPS + 1] is increased @@ -80,7 +92,7 @@ __gcov_pow2_profiler (gcov_type *counters, gcov_type value) } #endif -#ifdef L_gcov_pow2_profiler_atomic +#if defined(L_gcov_pow2_profiler_atomic) && GCOV_SUPPORTS_ATOMIC /* If VALUE is a power of two, COUNTERS[1] is incremented. Otherwise COUNTERS[0] is incremented. Function is thread-safe. */ @@ -134,7 +146,7 @@ __gcov_one_value_profiler (gcov_type *counters, gcov_type value) } #endif -#ifdef L_gcov_one_value_profiler_atomic +#if defined(L_gcov_one_value_profiler_atomic) && GCOV_SUPPORTS_ATOMIC /* Update one value profilers (COUNTERS) for a given VALUE. @@ -342,6 +354,7 @@ __gcov_time_profiler (gcov_type* counters) counters[0] = ++function_counter; } +#if GCOV_SUPPORTS_ATOMIC /* Sets corresponding COUNTERS if there is no value. Function is thread-safe. */ @@ -352,6 +365,7 @@ __gcov_time_profiler_atomic (gcov_type* counters) counters[0] = __atomic_add_fetch (&function_counter, 1, MEMMODEL_RELAXED); } #endif +#endif #ifdef L_gcov_average_profiler @@ -366,7 +380,7 @@ __gcov_average_profiler (gcov_type *counters, gcov_type value) } #endif -#ifdef L_gcov_average_profiler_atomic +#if defined(L_gcov_average_profiler_atomic) && GCOV_SUPPORTS_ATOMIC /* Increase corresponding COUNTER by VALUE. FIXME: Perhaps we want to saturate up. Function is thread-safe. */ @@ -388,7 +402,7 @@ __gcov_ior_profiler (gcov_type *counters, gcov_type value) } #endif -#ifdef L_gcov_ior_profiler_atomic +#if defined(L_gcov_ior_profiler_atomic) && GCOV_SUPPORTS_ATOMIC /* Bitwise-OR VALUE into COUNTER. Function is thread-safe. */ void -- 2.9.2 --------------573245037DB7C61514F91303--