r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.
Jonas Hahnfeld via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 16 11:19:29 PDT 2018
[ 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list