Message ID | CAF1bQ=R_qgrDHRyOKNr0eaN9BFOWku4bR6tFJeSRz6hoSCrjqQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Wed, Nov 20, 2013 at 10:44 AM, Rong Xu <xur@google.com> wrote: > On Tue, Nov 19, 2013 at 5:11 PM, Andrew Pinski <pinskia@gmail.com> wrote: >> On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu <xur@google.com> 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=<gcc_install_patch>/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). Thanks, Andrew Pinski > > I think that's the same reason we set the library path in > testsuite/lib/atomic-dg.exp, because <gcc_install_patch>/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 <xur@google.com> 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 <rth@redhat.com> 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~
On Wed, Nov 20, 2013 at 10:48 AM, Andrew Pinski <pinskia@gmail.com> wrote: > On Wed, Nov 20, 2013 at 10:44 AM, Rong Xu <xur@google.com> wrote: >> On Tue, Nov 19, 2013 at 5:11 PM, Andrew Pinski <pinskia@gmail.com> wrote: >>> On Tue, Nov 19, 2013 at 5:02 PM, Rong Xu <xur@google.com> 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=<gcc_install_patch>/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 <gcc_install_patch>/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 <xur@google.com> 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 <rth@redhat.com> 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~
On Wed, 20 Nov 2013, Rong Xu wrote: > 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. I think we should generally link libatomic with --as-needed by default on platforms supporting --as-needed, in line with the general principle that C code just using language not library facilities (_Atomic in this case) shouldn't need any special options to link it (libatomic is like libgcc, which is linked in automatically); the trickier question is what to do with it on any systems supporting shared libraries but not --as-needed.
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