Message ID | CAF1bQ=SOEgpD7DcWQGcWs4HZo2HOB-c6Qtus3bssS3DL8UMEBw@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Wed, Nov 20, 2013 at 2:07 PM, Rong Xu <xur@google.com> wrote: > Joseph and Andrew, thanks for the suggestion. That's really helpful. > > Here is the new patch for gcc.c. > Basically, it's just what you have suggested: enclosing -latomic with > --as-needed, and using macros. > For the case of no --as-needed support, I use static link. (just found > that some code already using this in the SPEC). > I'm flexible on this part -- if you think this is unnecessary, I can remove. I think Joseph's suggestion was also to include -latomic even when not generating atomic profiling due to the C11 code requiring it. Thanks, Andrew > > Thanks, > > -Rong > > Index: gcc.c > =================================================================== > --- gcc.c (revision 205053) > +++ gcc.c (working copy) > @@ -748,6 +748,23 @@ proper position among the other output files. */ > %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start > -u_vtable_map_vars_end}}" > #endif > > +/* This spec is for linking in libatomic in gcov atomic counter update. > + We will use the atomic functions defined in libatomic, only when the builtin > + versions are not available. In the case of no LD_AS_NEEDED support, we > + link libatomic statically. */ > + > +#ifndef GCOV_ATOMIC_SPEC > +#if USE_LD_AS_NEEDED > +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_AS_NEEDED_OPTION \ > + " -latomic} " LD_NO_AS_NEEDED_OPTION > +#elif defined(HAVE_LD_STATIC_DYNAMIC) > +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_STATIC_OPTION \ > + " -latomic " LD_DYNAMIC_OPTION "}" > +#else /* !USE_LD_AS_NEEDED && !HAVE_LD_STATIC_DYNAMIC */ > +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:-latomic}" > +#endif > +#endif > + > /* -u* was put back because both BSD and SysV seem to support it. */ > /* %{static:} simply prevents an error message if the target machine > doesn't handle -static. */ > @@ -771,7 +788,8 @@ proper position among the other output files. */ > %{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\ > + " GCOV_ATOMIC_SPEC "} " SANITIZER_SPEC " \ > %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\ > %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}" > > > > On Wed, Nov 20, 2013 at 1:04 PM, Joseph S. Myers > <joseph@codesourcery.com> wrote: >> 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. >> >> -- >> Joseph S. Myers >> joseph@codesourcery.com
OK. Sorry for miss-reading the message. In that case, linking in libatomic becomes a separate issue. We don't need to touch gcc.c in this patch. Thanks, -Rong On Wed, Nov 20, 2013 at 2:19 PM, Andrew Pinski <pinskia@gmail.com> wrote: > On Wed, Nov 20, 2013 at 2:07 PM, Rong Xu <xur@google.com> wrote: >> Joseph and Andrew, thanks for the suggestion. That's really helpful. >> >> Here is the new patch for gcc.c. >> Basically, it's just what you have suggested: enclosing -latomic with >> --as-needed, and using macros. >> For the case of no --as-needed support, I use static link. (just found >> that some code already using this in the SPEC). >> I'm flexible on this part -- if you think this is unnecessary, I can remove. > > > I think Joseph's suggestion was also to include -latomic even when not > generating atomic profiling due to the C11 code requiring it. > > Thanks, > Andrew > >> >> Thanks, >> >> -Rong >> >> Index: gcc.c >> =================================================================== >> --- gcc.c (revision 205053) >> +++ gcc.c (working copy) >> @@ -748,6 +748,23 @@ proper position among the other output files. */ >> %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start >> -u_vtable_map_vars_end}}" >> #endif >> >> +/* This spec is for linking in libatomic in gcov atomic counter update. >> + We will use the atomic functions defined in libatomic, only when the builtin >> + versions are not available. In the case of no LD_AS_NEEDED support, we >> + link libatomic statically. */ >> + >> +#ifndef GCOV_ATOMIC_SPEC >> +#if USE_LD_AS_NEEDED >> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_AS_NEEDED_OPTION \ >> + " -latomic} " LD_NO_AS_NEEDED_OPTION >> +#elif defined(HAVE_LD_STATIC_DYNAMIC) >> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_STATIC_OPTION \ >> + " -latomic " LD_DYNAMIC_OPTION "}" >> +#else /* !USE_LD_AS_NEEDED && !HAVE_LD_STATIC_DYNAMIC */ >> +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:-latomic}" >> +#endif >> +#endif >> + >> /* -u* was put back because both BSD and SysV seem to support it. */ >> /* %{static:} simply prevents an error message if the target machine >> doesn't handle -static. */ >> @@ -771,7 +788,8 @@ proper position among the other output files. */ >> %{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\ >> + " GCOV_ATOMIC_SPEC "} " SANITIZER_SPEC " \ >> %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\ >> %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}" >> >> >> >> On Wed, Nov 20, 2013 at 1:04 PM, Joseph S. Myers >> <joseph@codesourcery.com> wrote: >>> 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. >>> >>> -- >>> Joseph S. Myers >>> joseph@codesourcery.com
Index: gcc.c =================================================================== --- gcc.c (revision 205053) +++ gcc.c (working copy) @@ -748,6 +748,23 @@ proper position among the other output files. */ %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start -u_vtable_map_vars_end}}" #endif +/* This spec is for linking in libatomic in gcov atomic counter update. + We will use the atomic functions defined in libatomic, only when the builtin + versions are not available. In the case of no LD_AS_NEEDED support, we + link libatomic statically. */ + +#ifndef GCOV_ATOMIC_SPEC +#if USE_LD_AS_NEEDED +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_AS_NEEDED_OPTION \ + " -latomic} " LD_NO_AS_NEEDED_OPTION +#elif defined(HAVE_LD_STATIC_DYNAMIC) +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:" LD_STATIC_OPTION \ + " -latomic " LD_DYNAMIC_OPTION "}" +#else /* !USE_LD_AS_NEEDED && !HAVE_LD_STATIC_DYNAMIC */ +#define GCOV_ATOMIC_SPEC "%{fprofile-generate-atomic=*:-latomic}" +#endif +#endif + /* -u* was put back because both BSD and SysV seem to support it. */ /* %{static:} simply prevents an error message if the target machine doesn't handle -static. */ @@ -771,7 +788,8 @@ proper position among the other output files. */ %{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\ + " GCOV_ATOMIC_SPEC "} " SANITIZER_SPEC " \ %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\ %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"