[flang] Link to libatomic with openmp and rtlib=libgcc

[DavidTruby]

Currently when using OpenMP atomics we depend on some symbols from
libatomic. These symbols are provided in a separate library for the
libgcc runtime, so we should link to that when rtlib=libgcc.

For the compiler-rt case, the presence and location of the symbols is
dependent on how compiler-rt itself was built so we cannot make that
decision for the user. As such no extra flags are added in that case.

[llvmbot]

@llvm/pr-subscribers-flang-driver

@llvm/pr-subscribers-clang

Author: David Truby (DavidTruby)

Changes

Currently when using OpenMP atomics we depend on some symbols from
libatomic. These symbols are provided in a separate library for the
libgcc runtime, so we should link to that when rtlib=libgcc.

For the compiler-rt case, the presence and location of the symbols is
dependent on how compiler-rt itself was built so we cannot make that
decision for the user. As such no extra flags are added in that case.

---

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

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+10)
• (added) flang/test/Driver/atomic.f90 (+5)

diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 0c6a585c3acffd..7f9fc3559fcbd8 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1294,6 +1294,16 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
     CmdArgs.push_back("-lFortranRuntime");
     CmdArgs.push_back("-lFortranDecimal");
   }
+
+  // libomp needs libatomic for atomic operations if using libgcc
+  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+                   options::OPT_fno_openmp, false)) {
+    Driver::OpenMPRuntimeKind OMPRuntime =
+        TC.getDriver().getOpenMPRuntime(Args);
+    ToolChain::RuntimeLibType RuntimeLib = TC.GetRuntimeLibType(Args);
+    if (OMPRuntime == Driver::OMPRT_OMP && RuntimeLib == ToolChain::RLT_Libgcc)
+      CmdArgs.push_back("-latomic");
+  }
 }
 
 void tools::addFortranRuntimeLibraryPath(const ToolChain &TC,
diff --git a/flang/test/Driver/atomic.f90 b/flang/test/Driver/atomic.f90
new file mode 100644
index 00000000000000..4d3ba89be27580
--- /dev/null
+++ b/flang/test/Driver/atomic.f90
@@ -0,0 +1,5 @@
+!RUN: %flang -fopenmp -rtlib=libgcc -### %s 2>&1 | FileCheck --check-prefixes=GCC %s
+!RUN: %flang -fopenmp -rtlib=compiler-rt -### %s 2>&1 | FileCheck --check-prefixes=CRT %s
+
+!GCC: -latomic
+!CRT-NOT: -latomic

[llvmbot]

@llvm/pr-subscribers-clang-driver

Author: David Truby (DavidTruby)

Changes

Currently when using OpenMP atomics we depend on some symbols from
libatomic. These symbols are provided in a separate library for the
libgcc runtime, so we should link to that when rtlib=libgcc.

For the compiler-rt case, the presence and location of the symbols is
dependent on how compiler-rt itself was built so we cannot make that
decision for the user. As such no extra flags are added in that case.

---

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

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+10)
• (added) flang/test/Driver/atomic.f90 (+5)

diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 0c6a585c3acffd..7f9fc3559fcbd8 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1294,6 +1294,16 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
     CmdArgs.push_back("-lFortranRuntime");
     CmdArgs.push_back("-lFortranDecimal");
   }
+
+  // libomp needs libatomic for atomic operations if using libgcc
+  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+                   options::OPT_fno_openmp, false)) {
+    Driver::OpenMPRuntimeKind OMPRuntime =
+        TC.getDriver().getOpenMPRuntime(Args);
+    ToolChain::RuntimeLibType RuntimeLib = TC.GetRuntimeLibType(Args);
+    if (OMPRuntime == Driver::OMPRT_OMP && RuntimeLib == ToolChain::RLT_Libgcc)
+      CmdArgs.push_back("-latomic");
+  }
 }
 
 void tools::addFortranRuntimeLibraryPath(const ToolChain &TC,
diff --git a/flang/test/Driver/atomic.f90 b/flang/test/Driver/atomic.f90
new file mode 100644
index 00000000000000..4d3ba89be27580
--- /dev/null
+++ b/flang/test/Driver/atomic.f90
@@ -0,0 +1,5 @@
+!RUN: %flang -fopenmp -rtlib=libgcc -### %s 2>&1 | FileCheck --check-prefixes=GCC %s
+!RUN: %flang -fopenmp -rtlib=compiler-rt -### %s 2>&1 | FileCheck --check-prefixes=CRT %s
+
+!GCC: -latomic
+!CRT-NOT: -latomic

[mjklemm]

LGTM, I think this simplifies using the compiler a bit when it comes to OpenMP and more complex atomic regions.

[tblah]

LGTM thanks!

[NimishMishra]

LGTM! Thanks for this.

The Windows CI failure seems to be in Driver/atomic.f90. Do we disable this test on Windows (if there is a way to do so)?

[Meinersbur]

@NimishMishra Better way would be to prescribe target and linker: -target i686-pc-linux-gnu -fuse-ld=ld

[Meinersbur]

LGTM