[ Moving discussion from https://reviews.llvm.org/D49386 to the relevant
comment on cfe-commits, CC'ing Hal who commented on the original issue ]
Is this change really a good idea? It always requires libatomic for all
OpenMP applications, even if there is no 'omp atomic' directive or all
of them can be lowered to atomic instructions that don't require a
runtime library. I'd argue that it's a larger restriction than the
problem it solves.
Per https://clang.llvm.org/docs/Toolchain.html#libatomic-gnu the user is
expected to manually link -latomic whenever Clang can't lower atomic
instructions - including C11 atomics and C++ atomics. In my opinion
OpenMP is just another abstraction that doesn't require a special
treatment.
Thoughts?
Jonas
On 2018-07-06 23:13, Alexey Bataev via cfe-commits wrote:
Author: abataev
Date: Fri Jul 6 14:13:41 2018
New Revision: 336467
URL: http://llvm.org/viewvc/llvm-project?rev=336467&view=rev
Log:
[OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.
On Linux atomic constructs in OpenMP require libatomic library. Patch
links libatomic when -fopenmp is used.
Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/OpenMP/linking.c
Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=336467&r1=336466&r2=336467&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Fri Jul 6 14:13:41 2018
@@ -479,6 +479,7 @@ void tools::gnutools::Linker::ConstructJ
bool WantPthread = Args.hasArg(options::OPT_pthread) ||
Args.hasArg(options::OPT_pthreads);
+ bool WantAtomic = false;
// FIXME: Only pass GompNeedsRT = true for platforms with
libgomp that
// require librt. Most modern Linux platforms do, but some may
not.
@@ -487,13 +488,16 @@ void tools::gnutools::Linker::ConstructJ
/* GompNeedsRT= */ true))
// OpenMP runtimes implies pthreads when using the GNU
toolchain.
// FIXME: Does this really make sense for all GNU toolchains?
- WantPthread = true;
+ WantAtomic = WantPthread = true;
AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
if (WantPthread && !isAndroid)
CmdArgs.push_back("-lpthread");
+ if (WantAtomic)
+ CmdArgs.push_back("-latomic");
+
if (Args.hasArg(options::OPT_fsplit_stack))
CmdArgs.push_back("--wrap=pthread_create");
Modified: cfe/trunk/test/OpenMP/linking.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/linking.c?rev=336467&r1=336466&r2=336467&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/linking.c (original)
+++ cfe/trunk/test/OpenMP/linking.c Fri Jul 6 14:13:41 2018
@@ -8,14 +8,14 @@
// RUN: | FileCheck --check-prefix=CHECK-LD-32 %s
// CHECK-LD-32: "{{.*}}ld{{(.exe)?}}"
// CHECK-LD-32: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-LD-32: "-lpthread" "-lc"
+// CHECK-LD-32: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp -target x86_64-unknown-linux -rtlib=platform \
// RUN: | FileCheck --check-prefix=CHECK-LD-64 %s
// CHECK-LD-64: "{{.*}}ld{{(.exe)?}}"
// CHECK-LD-64: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-LD-64: "-lpthread" "-lc"
+// CHECK-LD-64: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp=libgomp -target i386-unknown-linux
-rtlib=platform \
@@ -27,7 +27,7 @@
// SIMD-ONLY2-NOT: liomp
// CHECK-GOMP-LD-32: "{{.*}}ld{{(.exe)?}}"
// CHECK-GOMP-LD-32: "-lgomp" "-lrt"
-// CHECK-GOMP-LD-32: "-lpthread" "-lc"
+// CHECK-GOMP-LD-32: "-lpthread" "-latomic" "-lc"
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1
-fopenmp-simd -target i386-unknown-linux -rtlib=platform | FileCheck
--check-prefix SIMD-ONLY2 %s
// SIMD-ONLY2-NOT: lgomp
@@ -39,21 +39,21 @@
// RUN: | FileCheck --check-prefix=CHECK-GOMP-LD-64 %s
// CHECK-GOMP-LD-64: "{{.*}}ld{{(.exe)?}}"
// CHECK-GOMP-LD-64: "-lgomp" "-lrt"
-// CHECK-GOMP-LD-64: "-lpthread" "-lc"
+// CHECK-GOMP-LD-64: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp -target i386-unknown-linux -rtlib=platform \
// RUN: | FileCheck --check-prefix=CHECK-IOMP5-LD-32 %s
// CHECK-IOMP5-LD-32: "{{.*}}ld{{(.exe)?}}"
// CHECK-IOMP5-LD-32: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-IOMP5-LD-32: "-lpthread" "-lc"
+// CHECK-IOMP5-LD-32: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp -target x86_64-unknown-linux -rtlib=platform \
// RUN: | FileCheck --check-prefix=CHECK-IOMP5-LD-64 %s
// CHECK-IOMP5-LD-64: "{{.*}}ld{{(.exe)?}}"
// CHECK-IOMP5-LD-64: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-IOMP5-LD-64: "-lpthread" "-lc"
+// CHECK-IOMP5-LD-64: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp=lib -target i386-unknown-linux \
@@ -71,7 +71,7 @@
// RUN: | FileCheck --check-prefix=CHECK-LD-OVERRIDE-32 %s
// CHECK-LD-OVERRIDE-32: "{{.*}}ld{{(.exe)?}}"
// CHECK-LD-OVERRIDE-32: "-lgomp" "-lrt"
-// CHECK-LD-OVERRIDE-32: "-lpthread" "-lc"
+// CHECK-LD-OVERRIDE-32: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp -fopenmp=libgomp -target x86_64-unknown-linux \
@@ -79,7 +79,7 @@
// RUN: | FileCheck --check-prefix=CHECK-LD-OVERRIDE-64 %s
// CHECK-LD-OVERRIDE-64: "{{.*}}ld{{(.exe)?}}"
// CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt"
-// CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
+// CHECK-LD-OVERRIDE-64: "-lpthread" "-latomic" "-lc"
//
// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
// RUN: -fopenmp=libomp -target x86_64-msvc-win32 -rtlib=platform
\
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
