[Flang][driver] Do not emit -latomic on link line on Windows

[mjklemm]

Flang on Windows added -latomic to the link line. This library does not exist on Windows and the linker gives a warning.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Michael Klemm (mjklemm)

Changes

Flang on Windows added -latomic to the link line. This library does not exist on Windows and the linker gives a warning.

---

Full diff: https://github.com/llvm/llvm-project/pull/164648.diff

1 Files Affected:

• (modified) clang/lib/Driver/ToolChain.cpp (+4-1)

diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3d5cac62afe01..b5496e6300ba9 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -851,8 +851,11 @@ void ToolChain::addFortranRuntimeLibs(const ArgList &Args,
                    options::OPT_fno_openmp, false)) {
     Driver::OpenMPRuntimeKind OMPRuntime = getDriver().getOpenMPRuntime(Args);
     ToolChain::RuntimeLibType RuntimeLib = GetRuntimeLibType(Args);
-    if (OMPRuntime == Driver::OMPRT_OMP && RuntimeLib == ToolChain::RLT_Libgcc)
+    if ((OMPRuntime == Driver::OMPRT_OMP &&
+         RuntimeLib == ToolChain::RLT_Libgcc) &&
+        !getTriple().isKnownWindowsMSVCEnvironment()) {
       CmdArgs.push_back("-latomic");
+    }
   }
 }
 

[Meinersbur]

LGTM

[DavidTruby]

Am I right in thinking this is only for MinGW targets and not msvcrt ones? I.e. with a triple like x86_64-pc-windows-gnu instead of x86_64-pc-windows-msvc; I think in the latter we don't add this already.
If so, I think it looks fine but I wonder where you're going to get the atomic symbols from in that case? Will programs that need them that use this triple just not work? (Or not work without -rtlib=compiler-rt I suppose)

Edit: Oddly, I do seem to be getting -latomic added to my link line (without this patch) even on aarch64-pc-windows-msvc, it just prints a warning and ignores it. But on Windows on Arm there is no libgcc so we shouldn't have RuntimeLib == ToolChain::RLT_Libgcc. That might be a larger bug we need to look at for the msvc triples...

[Meinersbur]

I do have libatomic.a in my MinGW installation. The patch should only apply to MSVC build enviroment (cl.exe and link.exe). The command line switch wouldn't be -l anyway. The summary mentioned it was only a warning anyway.

Since LLVM may emit libgcc-style atomic calls, the only things that could provide them would be compiler-rt (or ucrt equivalents the are still unknown to LLVM: #134455). These are disabled by default, COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=On:

llvm-project/compiler-rt/lib/builtins/CMakeLists.txt

Lines 232 to 234 in ece83ed

	option(COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN
	"Skip the atomic builtin (these should normally be provided by a shared library)"
	On)

There also is an option to create an equivalent to libatomic: clang_rt.atomic.lib (COMPILER_RT_BUILD_STANDALONE_LIBATOMIC), even on Windows (I think).

See previous discussion: https://discourse.llvm.org/t/atomics-on-windows/84329

[DavidTruby]

I haven't done anything special to turn on atomic support in compiler-rt and I can build and run at least simple programs using omp atomic and get the expected results. Not sure where I'm getting the symbols from.

Anyway, if this is just for lld-link lines we definitely shouldn't be adding this at all so LGTM!

[Meinersbur]

I haven't done anything special to turn on atomic support in compiler-rt and I can build and run at least simple programs using omp atomic and get the expected results. Not sure where I'm getting the symbols from.

LLVM emits CPU instructions for supported cases. A call to a runtime function (should¹) only happens as a fallback, e.g. when the number of bytes that needs to be atomically written is not a constant 1, 2, 4, or 8. msvc does not support int128_t, so the case of integer(16) (or complex(8)) falls outside what Microsoft's msvc/ucrt could handle, at least before its C11 atomics support which implements its own builtins such as __c11_atomic_store. LLVM just needs to know about them.

Footnotes

1. Currently Flang determines this on the data type, not by byte length. So you may see atomic calls even when not strictly necessary. [Transforms] Introduce BuildBuiltins.h atomic helpers #134455 was meant to fix this. ↩