[RFC] PR81358: Enable automatic linking of libatomic
Prathamesh Kulkarni
prathameshk@nvidia.com
Fri Jan 10 04:17:52 GMT 2025
> -----Original Message-----
> From: Thomas Schwinge <tschwinge@baylibre.com>
> Sent: 07 January 2025 17:45
> To: Prathamesh Kulkarni <prathameshk@nvidia.com>
> Cc: Tobias Burnus <tburnus@baylibre.com>; Joseph Myers
> <josmyers@redhat.com>; Xi Ruoyao <xry111@xry111.site>; Matthew
> Malcomson <mmalcomson@nvidia.com>; gcc-patches@gcc.gnu.org; Tom de
> Vries <tdevries@suse.de>
> Subject: RE: [RFC] PR81358: Enable automatic linking of libatomic
>
> External email: Use caution opening links or attachments
>
>
> Hi Prathamesh!
Hi Thomas, thanks for the review!
>
> Thanks for working on this!
>
>
> Per my understanding, this patch won't automagically resolve the need
> to
> (occasionally) having to specify '-foffload-options=nvptx-none=-
> latomic'
> for nvptx offloading: it doesn't use 'LINK_LIBATOMIC_SPEC', currently
> only used via 'GNU_USER_TARGET_LINK_GCC_C_SEQUENCE_SPEC' from
> 'gcc/config/gnu-user.h' (general issue, affecting a lot of
> configurations, to be addressed as necessary):
>
> > --- a/gcc/config/gnu-user.h
> > +++ b/gcc/config/gnu-user.h
>
> > #define GNU_USER_TARGET_LINK_GCC_C_SEQUENCE_SPEC \
> > - "%{static|static-pie:--start-group} %G %{!nolibc:%L} \
> > + "%{static|static-pie:--start-group} %G %{!nolibc:"
> > + LINK_LIBATOMIC_SPEC "%L} \
> > %{static|static-pie:--end-group}%{!static:%{!static-pie:%G}}"
>
> > --- a/gcc/gcc.cc
> > +++ b/gcc/gcc.cc
>
> > /* Here is the spec for running the linker, after compiling all
> > files. */
> >
> > +#if defined(TARGET_PROVIDES_LIBATOMIC) && defined(USE_LD_AS_NEEDED)
> > +#define LINK_LIBATOMIC_SPEC "%{!fno-link-libatomic:"
> LD_AS_NEEDED_OPTION \
> > + " -latomic " LD_NO_AS_NEEDED_OPTION "} "
> > +#else
> > +#define LINK_LIBATOMIC_SPEC ""
> > +#endif
> > +
> > /* This is overridable by the target in case they need to specify
> the
> > -lgcc and -lc order specially, yet not require them to override
> all
> > of LINK_COMMAND_SPEC. */
>
> ..., and the nvptx linker doesn't support '--as-needed'.
>
> I'll think about it; indeed it'd be good to get that resolved, too.
>
>
> On 2024-12-20T15:37:42+0000, Prathamesh Kulkarni
> <prathameshk@nvidia.com> wrote:
> > [...] copying libatomic.a over to $(gcc_objdir)$(MULTISUBDIR)/, and
> > can confirm that 64-bit libatomic.a is copied to $build/gcc/ and 32-
> bit libatomic.a is copied to $build/gcc/32/.
>
> So this:
>
> > --- a/libatomic/Makefile.am
> > +++ b/libatomic/Makefile.am
>
> > @@ -162,6 +162,11 @@ libatomic_convenience_la_LIBADD =
> > $(libatomic_la_LIBADD) # when it is reloaded during the build of
> all-multi.
> > all-multi: $(libatomic_la_LIBADD)
> >
> > +gcc_objdir = $(MULTIBUILDTOP)../../$(host_subdir)/gcc
> > +all: all-multi libatomic.la libatomic_convenience.la
> > + $(INSTALL_DATA) .libs/libatomic.a $(gcc_objdir)$(MULTISUBDIR)/
> > + chmod 644 $(gcc_objdir)$(MULTISUBDIR)/libatomic.a
>
> ... is conceptually modelled after libgcc, where the libraries get
> copied into 'gcc/'? However, here we only copy the static
> 'libatomic.a'; libgcc does a 'make install-leaf', see
> 'libgcc/Makefile.in':
>
> all: all-multi
> # Now that we have built all the objects, we need to copy
> # them back to the GCC directory. Too many things (other
> # in-tree libraries, and DejaGNU) know about the layout
> # of the build tree, for now.
> $(MAKE) install-leaf DESTDIR=$(gcc_objdir) \
> slibdir= libsubdir= MULTIOSDIR=$(MULTIDIR)
>
> ..., which also installs dynamic libraries. Is that difference
> intentional and/or possibly important?
Well, I wasn't sure what extension to use for shared libraries, and initially avoided copying them.
libgcc seems to use $(SHLIB_EXT) to specify extension name for shared libraries, which can be overridden
by targets.
Matthew pointed out to me that using libtool --mode=install works for copying both
static and shared libraries (with the numbered version libatomic.so.1.2.0), so in the attached patch,
I changed Makefile.am rule to following:
gcc_objdir = `pwd`/$(MULTIBUILDTOP)../../gcc/
all: all-multi libatomic.la libatomic_convenience.la
$(LIBTOOL) --mode=install $(INSTALL_DATA) libatomic.la $(gcc_objdir)$(MULTISUBDIR)/
Which seems to install libatomic.a, libatomic.so and the numbered version in $build/gcc/ and in $build/gcc/32/
(and $build/gcc/mgomp/ for nvptx build).
Does it look OK ?
>
> Does libatomic even need a switch corresponding to '-static-libgcc'?
I am not sure, hoping for Joseph to chime in.
>
> Given that libatomic libraries get copied into 'gcc/', will we be able
> (later, incrementally) to remove some setup code from the test suites'
> '*.exp' files, to locate build-tree libatomic?
I'd guess so.
>
> Also, given the presumed similarity to how libgcc is handled (with, of
> course, the difference that libatomic isn't built for all
> configurations), should we maybe in the build system place the new
> libatomic handling next to the existing libgcc handling?
> Specifically, instead of:
>
> > --- a/Makefile.def
> > +++ b/Makefile.def
>
> > +dependencies = { module=configure-target-libbacktrace;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libgloss; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-newlib;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libgomp; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libitm;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libstdc++v3; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libsanitizer;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libvtv; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libssp;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libquadmath; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libgfortran;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libffi; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libobjc;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libada; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libgm2;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libgo; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-libgrust;
> > +on=all-target-libatomic; }; dependencies = {
> > +module=configure-target-libphobos; on=all-target-libatomic; };
> > +dependencies = { module=configure-target-zlib;
> > +on=all-target-libatomic; };
>
> ... handle libatomic like:
>
> // [...] By default target modules depend
> // on libgcc and newlib/libgloss.
The patch adjusts Makefile.tpl to add no_atomic to lang_env_dependencies, and adding dependency on libatomic if the attribute
is not set for target library, similar to others (no_gcc, no_c). This also fixes the newlib failure with offloading.
Does it look OK ?
>
> ..., and regarding:
>
> > --- a/configure.ac
> > +++ b/configure.ac
>
> > +# If we are building libatomic, bootstrap it.
> > +if echo " ${target_configdirs} " | grep " libatomic " > /dev/null
> > +2>&1 ; then
> > + bootstrap_target_libs=${bootstrap_target_libs}target-libatomic,
> > +fi
>
> ..., maybe place that right after:
>
> # Target libraries that we bootstrap.
> bootstrap_target_libs=,target-libgcc,
>
> But I haven't spent a lot of thought on these items, so maybe that
> doesn't make sense.
>
>
> > (2) libatomic_convenience.la was not getting generated for some
> reason, which resulted in build failure while building libdruntime.
> > The patch adds libatomic_convenience.la as a dependency, and I can
> see it now getting generated, which seems to fix the build issue with
> libdruntime.
>
> It's not obvious to me why 'libatomic_convenience' belongs onto this
> 'all' rule, given that we don't do anything with
> 'libatomic_convenience'
> there?
Because it broke libdruntime. Quoting from my previous reply:
"libatomic_convenience.la was not getting generated for some reason, which resulted in build failure while building libdruntime.
The patch adds libatomic_convenience.la as a dependency, and I can see it now getting generated, which seems to fix the build issue with libdruntime."
>
>
> > Patch passes bootstrap+test with multilib enabled for --enable-
> languages=all on x86_64-linux-gnu, and for --enable-
> languages=c,c++,fortran on aarch64-linux-gnu.
> > Does this version look OK ?
>
> For nvptx target, with newlib sources sym-linked into a combined tree,
> the build fails:
>
> [...]
> make[2]: Leaving directory
> '/home/thomas/tmp/source/gcc/build/queue-slim-omp/build-gcc-offload-
> nvptx-none/nvptx-none/libgcc'
> make[1]: Circular configure-target-libatomic <- maybe-all-target-
> newlib dependency dropped.
> Checking multilib configuration for libatomic...
> mkdir -p -- nvptx-none/libatomic
> Configuring in nvptx-none/libatomic
> [...]
> checking whether the C compiler works... no
> configure: error: in `/home/thomas/tmp/source/gcc/build/queue-
> slim-omp/build-gcc-offload-nvptx-none/nvptx-none/libatomic':
> configure: error: C compiler cannot create executables
> [...]
>
> This is, per my understanding, because libatomic is attempted to be
> built before newlib, but the former depends on the latter.
>
> So, the "Circular [...] dependency" will need resolving, I suppose?
Fixed by the above change.
>
>
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
>
> > +flink-libatomic
> > +Common Driver Var(flag_link_libatomic) Init(1)
>
> 'gcc/common.opt.urls' needs updating, I suppose?
Done, thanks.
>
>
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -206,7 +206,7 @@ in the following sections.
> > -fpermitted-flt-eval-methods=@var{standard}
> > -fplan9-extensions -fsigned-bitfields -funsigned-bitfields
> > -fsigned-char -funsigned-char -fstrict-flex-arrays[=@var{n}]
> > --fsso-struct=@var{endianness}}
> > +-flink-libatomic -fsso-struct=@var{endianness}}
> >
> > @item C++ Language Options
> > @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
> > @@ -2899,6 +2899,10 @@ The @option{-fstrict_flex_arrays} option
> > interacts with the @option{-Wstrict-flex-arrays} option.
> > @xref{Warning Options}, for more information.
> >
> > +@opindex flink-libatomic
> > +@item -flink-libatomic
> > +Enable linking of libatomic if it's supported by target. Enabled by
> default.
> > +
> > @opindex fsso-struct
> > @item -fsso-struct=@var{endianness}
> > Set the default scalar storage order of structures and unions to
> the
>
> Why place '-flink-libatomic' between '-fstrict-flex-arrays' and '-
> fsso-struct'?
Moved it to end.
>
> Also, if enabled by default, don't we usually describe the negative
> form
> ('-fno-link-libatomic') in the manual?
Fixed, thanks.
I am re-validating the patch for following configs:
(a) Bootstrap+test with multilib enabled for all languages on x86_64-linux-gnu.
(b) Bootstrap+test for c,c++,fortran on aarch64-linux-gnu.
(c) Cross testing on x86_64->aarch64.
(d) Test offloading with nvptx.
Does the patch look OK if it passes testing for above configs ?
Thanks,
Prathamesh
>
>
> Grüße
> Thomas
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: p-306-16.txt
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20250110/3add8d69/attachment-0001.txt>
More information about the Gcc-patches
mailing list