[RFC] Automatic linking of libatomic via gcc.c or ...? [PR81358] (dependency for libgomp on nvptx dep, configure overriddable, ...)

Hi all, hi Joseph & Jakub,

BACKGROUND:

The main reason I am interested in this is offloading where
OpenACC/OpenMP code might run into:
     unresolved symbol __atomic_compare_exchange_16
and it is nontrivial to find out that the solution is:
     -foffload=nvptx-none=-latomic
And the atomic use can appear for innocent looking code.
(The dependency comes via libgomp/config/nvptx/atomic.c
for __sync_val_compare_and_swap_16.)

However, as PR81358 shows, also for normal C (and C++) code
it would be nice as atomics are part of the language.

Target specific: for RISC-V (with -as-needed support only
with pthread, otherwise unconditionally) – and seemingly for RTEMS –
-latomic is already linked via LIB_SPEC.


My initial plan was to add -latomic to libgomp.spec - if built
for that target and using -as-needed if available, cf. last but one
email in the thread
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/thread.html#555298
(my initial RFC question, quoted in the first email, was in September)

However, adding it there was opposed by Jakub as:
"I think for libgomp.spec we should add it solely for the offloading targets,
neither GCC generated code for OpenMP construct nor libgomp itself needs
-latomic on the hosts."
(Albeit for nvptx, see above.)


SOLUTION PART 1: Add configure option --enable-autolink-libatomic

[See attached patch (only configure check)]

Example for an explicit use: enable it for offloading targets
(which enables it for nvptx but not for amdgcn which does not
build libatomic). — And for the host, you may want to enable if
you always install libatomic and -as-needed is supported and
otherwise, you might want to disable it.

QUESTION:
* Should the default of --enable-autolink-libatomic default
   to 'yes' or 'no'?
* Should the default additionally depend on having -as-needed support?
   (Which would exclude nvptx with default settings.)


SOLUTION PART 2: Actually linking libatomic

?

QUESTION: I have no idea where to add the -latomic.

(a) Still do it in libgomp.spec but now with that configure
flag to be able to disable it?

Pro:
+ Solves problem for nvptx, which adds atomic code to
   libgomp/config/nvptx/atomic.c
+ rather minimal invasive and by tuning the config option
   it could be used when needed
Con:
- While OpenMP has atomics, libgomp by itself in general
   does not depend on libatomic for most targets
   - this can be mitigated by using the configure flag
   but it is not ideal, either.
- C code can use atomic directly and thus it would be nice
   if it could be automatically linked. (Especially if the
   target supports the -as-needed linker flag.)


(b) Do it in the driver

Pro:
+ will automatically work for C/C++ atomic code
+ with -as-needed it will only be linked if really needed
   (caveat: lib file has to be present at link time.)
Con:
- If -as-needed is not supported and it is always linked, this
   adds unneccessary dependencies (shared lib) or file size (static lib)
- Adding files for the linker to analyze does not help with
   the compile size
- The file has to be present, even if -as-needed is supported,
   adding a hard dependency on the libatomic library for Linux
   distros

For doing it in the driver, I am not sure when to add it.
Used:
- Direct consumer is C/C++ using atomics.
- For Fortran, it (currently) is only used for with
   nvptx offloading as described above.
- For offloading, the compilation/linking handling is
   slightly different and it needs to work there as well.
- No idea about Ada, D, or other direct or indirect dependencies


RISC-V + RTEMS use LIB_SPEC to add it.

One possibility would be to add it to the init_gcc_specs call,
but that feels like a sledge hammer solution.

Question: Where do you think should it be in the driver?

Other thoughts?

Or is solution (a) better? (That is: previous patch +
new --enable-autolink-libatomic for libgomp, only.)
Which is kind of a complicated nvptx-offloading-only solution?

Tobias

PS: I assume -static-libatomic then has to be added as well when we
add -latomic in the driver.

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

On Wed, 14 Oct 2020, Tobias Burnus wrote:

> Question: Where do you think should it be in the driver?

I think it should be somewhere in the expansion of %(link_gcc_c_sequence) 
(i.e. LINK_GCC_C_SEQUENCE_SPEC, which has various target-specific 
definitions), since that's what expands to something like -lgcc -lc -lgcc.  
Maybe after the first -lgcc and before the -lc, since libatomic has 
references to libc functions.

Hi Joseph, hi Jakub,

(a) For the driver route:

On 10/15/20 12:22 AM, Joseph Myers wrote:
> I think it should be somewhere in the expansion of %(link_gcc_c_sequence)
> (i.e. LINK_GCC_C_SEQUENCE_SPEC, which has various target-specific
> definitions), since that's what expands to something like -lgcc -lc -lgcc.
> Maybe after the first -lgcc and before the -lc, since libatomic has
> references to libc functions.

Lightly tested draft patch attached. (--enable-autolink-libatomic is 'no' by default)
Does this make sense, in principle?

When trying the patch, one runs into the problem that -latomic
fails in the target configure for libgomp ("error: C compiler
cannot create executables"). Any suggestion? (That's on x86_64-gnu-linux.)

(b) I start thinking that putting it into libgomp.spec – possibly with the new configure
--enable... flag and/or only for nvptx (which uses libatomic via
../libgomp/config/nvptx/atomic.c) – is the better solution.
Thoughts?

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

On Thu, 15 Oct 2020, Tobias Burnus wrote:

> Hi Joseph, hi Jakub,
> 
> (a) For the driver route:
> 
> On 10/15/20 12:22 AM, Joseph Myers wrote:
> > I think it should be somewhere in the expansion of %(link_gcc_c_sequence)
> > (i.e. LINK_GCC_C_SEQUENCE_SPEC, which has various target-specific
> > definitions), since that's what expands to something like -lgcc -lc -lgcc.
> > Maybe after the first -lgcc and before the -lc, since libatomic has
> > references to libc functions.
> 
> Lightly tested draft patch attached. (--enable-autolink-libatomic is 'no' by
> default)

I think the configure option should be on by default.

I'd expect only gcc.c to define LINK_LIBATOMIC_SPEC rather than 
duplicating it in gnu-user.h.

I'd expect either a default definition of LINK_LIBATOMIC_SPEC to "" when 
the configure option is disabled, or else conditionals in the macros that 
use LINK_LIBATOMIC_SPEC to avoid them using an undefined macro.

> When trying the patch, one runs into the problem that -latomic
> fails in the target configure for libgomp ("error: C compiler
> cannot create executables"). Any suggestion? (That's on x86_64-gnu-linux.)

Yes, making this change means you need to ensure the build of target 
libraries and testcases can find the in-build-tree libatomic (and that 
tests can find it at runtime as needed in the shared library case), just 
like it already needs to be able to find the in-build-tree libgcc.

In the case of libgcc, the libgcc build process copies the libraries into 
the host-side gcc/ object directory.  Maybe doing that for libatomic would 
be simpler than teaching lots of separate places how to find libatomic in 
the build tree (though for any tests that might use shared libatomic at 
runtime, it's still necessary to ensure that works for testing GCC).