> -----Original Message-----
> From: Prathamesh Kulkarni <prathame...@nvidia.com>
> Sent: 10 January 2025 09:48
> To: Thomas Schwinge <tschwi...@baylibre.com>
> Cc: Tobias Burnus <tbur...@baylibre.com>; Joseph Myers
> <josmy...@redhat.com>; Xi Ruoyao <xry...@xry111.site>; Matthew
> Malcomson <mmalcom...@nvidia.com>; gcc-patches@gcc.gnu.org; Tom de
> Vries <tdevr...@suse.de>
> Subject: RE: [RFC] PR81358: Enable automatic linking of libatomic
>
> External email: Use caution opening links or attachments
>
>
> > -----Original Message-----
> > From: Thomas Schwinge <tschwi...@baylibre.com>
> > Sent: 07 January 2025 17:45
> > To: Prathamesh Kulkarni <prathame...@nvidia.com>
> > Cc: Tobias Burnus <tbur...@baylibre.com>; Joseph Myers
> > <josmy...@redhat.com>; Xi Ruoyao <xry...@xry111.site>; Matthew
> > Malcomson <mmalcom...@nvidia.com>; gcc-patches@gcc.gnu.org; Tom de
> > Vries <tdevr...@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
> > <prathame...@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 ?
Hi Joseph,
Does the above patch in:
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673155.html look OK to
commit (altho it's stage-4 now) ?
It fixes all the fallouts observed so far (multilib, nvptx offloading,
libdruntime).
Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
> >
> >
> > Grüße
> > Thomas
