From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26233 invoked by alias); 20 Nov 2013 19:00:35 -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 26217 invoked by uid 89); 20 Nov 2013 19:00:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.5 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RDNS_NONE autolearn=no version=3.3.2 X-HELO: mail-wg0-f51.google.com Received: from Unknown (HELO mail-wg0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 20 Nov 2013 18:59:31 +0000 Received: by mail-wg0-f51.google.com with SMTP id l18so1246311wgh.6 for ; Wed, 20 Nov 2013 10:59:22 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.194.78.99 with SMTP id a3mr63044wjx.93.1384973962680; Wed, 20 Nov 2013 10:59:22 -0800 (PST) Received: by 10.217.119.193 with HTTP; Wed, 20 Nov 2013 10:59:22 -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 20:31:00 -0000 Message-ID: Subject: Re: atomic update of profile counters (issue7000044) From: Andrew Pinski To: Rong Xu Cc: Richard Henderson , Richard Biener , Xinliang David Li , Jan Hubicka , GCC Patches , reply@codereview.appspotmail.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg02600.txt.bz2 On Wed, Nov 20, 2013 at 10:48 AM, Andrew Pinski wrote: > On Wed, Nov 20, 2013 at 10:44 AM, Rong Xu wrote: >> 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 don't see this as an issue really as you have the same issue with > all the target libraries (not limited to libatomic or libgomp or > libgfortran). You also also use --as-needed/--no-as-needed wrapped around the -latomic too; USE_LD_AS_NEEDED is needed to be used to check for --as-needed support and LD_AS_NEEDED_OPTION/LD_NO_AS_NEEDED_OPTION should be used instead of directly --as-needed/--no-as-needed. > > Thanks, > Andrew Pinski > >> >> 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~