From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7520 invoked by alias); 20 Nov 2013 18:45:24 -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 7506 invoked by uid 89); 20 Nov 2013 18:45:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-oa0-f50.google.com Received: from Unknown (HELO mail-oa0-f50.google.com) (209.85.219.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 20 Nov 2013 18:44:14 +0000 Received: by mail-oa0-f50.google.com with SMTP id n16so1103947oag.9 for ; Wed, 20 Nov 2013 10:44:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=GE66eHxKEaTXuPMkDKXvRw/e+c3PZOgmecsBqO3hsdI=; b=c0aOU4bPk7wqSdfv2AkMZGn7XiXhSSHZEum2TyUq5cN4u3E9kT7sWEDnYcZ+MfPq0Y oPeY4dQZ5vhtlFmRXqrth8LvI2i7Ex+dsecM3s3WmbSIMAeiA93hr6IwU5DJuoytGWZs TcTSgI4RdNbvApAdyuKK1M+rP4s3F4eiQUFRtBj0dc5/nFnd7EjHlomLtBPNFB8Xou/Z PMnFZJfURK+WNvcTf7Ew+xDihXSZ3DW48F/b0KoHuoHpPH81cKQth7OuFNAgt1w7aeXS OvN1ux3zStffSL8C76PpOc4us5WVZ8CR000bmlDD8ajEuzDRRd3bcT5B8dSQGkr5VgVT H0qQ== X-Gm-Message-State: ALoCoQlzFwOs4v/aGBzg1aR6/kU/EsDl0h53L2Irw0DId0mjWoQmx2NuHw11ud3pggPP7N3a2qxd2OQqTKqyIPeGJYRugl+doh3fEbwQIubNhzS6HxlqKUKcniqhjCLrdwvhtQ2Hu2K+Uxf2K0Sw6QcXhVGLQSnnoEiX8+0wS8OkZ2EgTEldfT2zfWY1ANaEJJg/hOkGCO6jtuod7/YBkTY+BJmpt3h74g== MIME-Version: 1.0 X-Received: by 10.182.113.195 with SMTP id ja3mr1685043obb.46.1384973046466; Wed, 20 Nov 2013 10:44:06 -0800 (PST) Received: by 10.182.32.39 with HTTP; Wed, 20 Nov 2013 10:44:06 -0800 (PST) In-Reply-To: References: <20121221064539.0E1A7100704@rong.mtv.corp.google.com> <20121221092532.GA7055@kam.mff.cuni.cz> <50EB31B7.9090307@redhat.com> Date: Wed, 20 Nov 2013 19:59:00 -0000 Message-ID: Subject: Re: atomic update of profile counters (issue7000044) From: Rong Xu To: Andrew Pinski Cc: Richard Henderson , Richard Biener , Xinliang David Li , Jan Hubicka , GCC Patches , reply@codereview.appspotmail.com Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg02594.txt.bz2 On Tue, Nov 19, 2013 at 5:11 PM, Andrew Pinski wrote: > On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu wrote: >> Hi all, >> >> I merged this old patch with current trunk. I also make the following changes >> (1) not using weak references. Now every *profile_atomic() has it's >> own .o so that none of them will be in the final binary if >> -fprofile-generate-atomic is not specified. >> (2) more value profilers have the atomic version. >> (3) not link to libatomic. I used to link the libatomic in the >> presence of -fprofile-generate-atomic, per Andrew's suggestion. It >> used to work. But now if I can add -latomic in the SPEC, it cannot >> find the libatomic.so.1 (unless I specify the PATH). I did not find an >> easy way to statically link libatomic.a. Andrew: Do you have any >> suggestion? Or should we let the user link to libatomic.a if the >> builtins are not expanded? > > It should work for an installed GCC. For testing you might need > something that is included inside testsuite/lib/atomic-dg.exp which > sets the library path to include libatomic build directory. When I change the SPEC to include libatomic, the compiler can find libatomic. I.e. using >> gcc -O2 -fprofile-generate -fprofile-generate-atomic=3 hello_world.c generates a.out without any problem. But since there are both shared and static libatomic in lib64, it chooses to use the dynamic one. >> ldd a.out linux-vdso.so.1 => (0x00007fff56bff000) libatomic.so.1 => not found libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00002b0720261000) /lib64/ld-linux-x86-64.so.2 (0x00002b072003c000) >> ./a.out ./a.out: error while loading shared libraries: libatomic.so.1: cannot open shared object file: No such file or directory while >> LD_LIBRARY_PATH=/lib64 ./a.out works fine. I think that's the same reason we set the library path in testsuite/lib/atomic-dg.exp, because /lib64 is not in the dynamic library search list. I could do this in the SPEC -Wl,-Bstatic -latomic -Wl,-Bdynamic which would link libatomic statically. I works for me. But it looks a little weird in gcc driver. Index: gcc.c =================================================================== --- gcc.c (revision 205053) +++ gcc.c (working copy) @@ -771,7 +771,8 @@ %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\ %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\ %(mflib) " STACK_SPLIT_SPEC "\ - %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \ + %{fprofile-arcs|fprofile-generate*|coverage:-lgcov\ + %{fprofile-generate-atomic=*:-Wl,-Bstatic -latomic -Wl,-Bdynamic}} " SANITIZER_SPEC " \ %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\ %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}" #endif > I think now we require libatomic in more cases (C11 atomic support for > an example). > > Thanks, > Andrew Pinski > >> >> Is this OK for trunk? >> >> Thanks, >> >> -Rong >> >> On Mon, Jan 7, 2013 at 12:55 PM, Rong Xu wrote: >>> Function __gcov_indirect_call_profiler_atomic (which contains call to >>> the atomic function) is always emitted in libgcov. >>> Since we only link libatomic when -fprofile-gen-atomic is specified, >>> we have to make the atomic function weak -- otherwise, there is a >>> unsat for regular FDO gen build (of course, when the builtin is not >>> expanded). >>> >>> An alternative it to always link libatomic together with libgcov. Then >>> we don't need the weak stuff. I'm not sure when one is better. >>> >>> -Rong >>> >>> On Mon, Jan 7, 2013 at 12:36 PM, Richard Henderson wrote: >>>> On 01/03/2013 04:42 PM, Rong Xu wrote: >>>>> It links libatomic when -fprofile-gen-atomic is specified for FDO >>>>> instrumentation build. Here I assume libatomic is always installed. >>>>> Andrew: do you think if this is reasonable? >>>>> >>>>> It also disables the functionality if target does not support weak >>>>> (ie. TARGET_SUPPORTS_WEAK == 0). >>>> >>>> Since you're linking libatomic, you don't need weak references. >>>> >>>> I think its ok to assume libatomic is installed, given that the >>>> user has had to explicitly use the command-line option. >>>> >>>> >>>> r~