Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-23 Thread Jonas Hahnfeld via cfe-commits 

[ + cfe-commits ]

Reverted in r337722, I've reopened the bug.

Regards,
Jonas

On 2018-07-23 11:37, Alexey Bataev wrote:

Hi Jonas, yes, go ahead.

Best regards,
Alexey Bataev


23 июля 2018 г., в 5:16, Jonas Hahnfeld 

написал(а):



On 2018-07-23 11:08, Jonas Hahnfeld via cfe-commits wrote:

On 2018-07-19 20:55, Hal Finkel wrote:

On 07/19/2018 09:01 AM, Jonas Hahnfeld wrote:

On 2018-07-19 15:43, Hal Finkel wrote:

On 07/16/2018 01:19 PM, Jonas Hahnfeld wrote:
[ 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.


Can you please elaborate on why you feel that this is

problematic?


The linked patch deals with the case that there is no libatomic,
effectively disabling all tests of the OpenMP runtime (even

though

only few of them require atomic instructions). So apparently

there

are
Linux systems without libatomic. Taking them any chance to use

OpenMP

with Clang is a large regression IMO and not user-friendly

either.


If there's a significant population of such systems, then this
certainly
seems like a problem.

Let's revert this for now while we figure out what to do (which

might

just mean updating the documentation to include OpenMP where we

talk

about atomics).


Alexey, what do you think? Can I go ahead and revert this commit?

Thanks,
Jonas


Meh, my message got blocked by @hotmail.com :-( I hope you received

the

message(s) via the mailing list...

Regards,
Jonas


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-23 Thread Jonas Hahnfeld via cfe-commits 

On 2018-07-19 20:55, Hal Finkel wrote:

On 07/19/2018 09:01 AM, Jonas Hahnfeld wrote:

On 2018-07-19 15:43, Hal Finkel wrote:

On 07/16/2018 01:19 PM, Jonas Hahnfeld wrote:

[ 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.


Can you please elaborate on why you feel that this is problematic?


The linked patch deals with the case that there is no libatomic,
effectively disabling all tests of the OpenMP runtime (even though
only few of them require atomic instructions). So apparently there are
Linux systems without libatomic. Taking them any chance to use OpenMP
with Clang is a large regression IMO and not user-friendly either.


If there's a significant population of such systems, then this 
certainly

seems like a problem.

Let's revert this for now while we figure out what to do (which might
just mean updating the documentation to include OpenMP where we talk
about atomics).


Alexey, what do you think? Can I go ahead and revert this commit?

Thanks,
Jonas
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-19 Thread Hal Finkel via cfe-commits 


On 07/19/2018 09:01 AM, Jonas Hahnfeld wrote:
> On 2018-07-19 15:43, Hal Finkel wrote:
>> On 07/16/2018 01:19 PM, Jonas Hahnfeld wrote:
>>> [ 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.
>>
>> Can you please elaborate on why you feel that this is problematic?
>
> The linked patch deals with the case that there is no libatomic,
> effectively disabling all tests of the OpenMP runtime (even though
> only few of them require atomic instructions). So apparently there are
> Linux systems without libatomic. Taking them any chance to use OpenMP
> with Clang is a large regression IMO and not user-friendly either.

If there's a significant population of such systems, then this certainly
seems like a problem.

Let's revert this for now while we figure out what to do (which might
just mean updating the documentation to include OpenMP where we talk
about atomics).

>
>>> 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.
>>
>> From my perspective, because we instruct our users that all you need to
>> do in order to enable OpenMP is pass -fopenmp flags during compiling and
>> linking. The user should not need to know or care about how atomics are
>> implemented.
>>
>> It's not clear to me that our behavior for C++ atomics is good either.
>> From the documentation, it looks like the rationale is to avoid choosing
>> between the GNU libatomic implementation and the compiler-rt
>> implementation? We should probably make a default choice and provide a
>> flag to override. That would seem more user-friendly to me.
>
> I didn't mean to say it's a good default, but OpenMP is now different
> from C and C++. And as you said, the choice was probably made for a
> reason, so there should be some discussion whether to change it.

Agreed.

 -Hal

>
> Jonas

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-19 Thread Jonas Hahnfeld via cfe-commits 

On 2018-07-19 15:43, Hal Finkel wrote:

On 07/16/2018 01:19 PM, Jonas Hahnfeld wrote:

[ 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.


Can you please elaborate on why you feel that this is problematic?


The linked patch deals with the case that there is no libatomic, 
effectively disabling all tests of the OpenMP runtime (even though only 
few of them require atomic instructions). So apparently there are Linux 
systems without libatomic. Taking them any chance to use OpenMP with 
Clang is a large regression IMO and not user-friendly either.



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.


From my perspective, because we instruct our users that all you need to
do in order to enable OpenMP is pass -fopenmp flags during compiling 
and

linking. The user should not need to know or care about how atomics are
implemented.

It's not clear to me that our behavior for C++ atomics is good either.
From the documentation, it looks like the rationale is to avoid 
choosing

between the GNU libatomic implementation and the compiler-rt
implementation? We should probably make a default choice and provide a
flag to override. That would seem more user-friendly to me.


I didn't mean to say it's a good default, but OpenMP is now different 
from C and C++. And as you said, the choice was probably made for a 
reason, so there should be some discussion whether to change it.


Jonas
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-19 Thread Churbanov, Andrey via cfe-commits 
> Can you please elaborate on why you feel that this is problematic?

This change completely disables OpenMP parallelization on all systems those do 
not have libatomic library accessible.

So many people who earlier had working codes now cannot compiler any OpenMP 
program, even just print "Hello", that sound too huge restriction related to 
the fixed issue - save one compiler option for program with exotic atomic types.

-- Andrey

-Original Message-
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of Hal 
Finkel via cfe-commits
Sent: Thursday, July 19, 2018 4:44 PM
To: Jonas Hahnfeld ; Alexey Bataev 
Cc: cfe-commits@lists.llvm.org
Subject: Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is 
used.


On 07/16/2018 01:19 PM, Jonas Hahnfeld wrote:
> [ 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.

Can you please elaborate on why you feel that this is problematic?

> 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.

From my perspective, because we instruct our users that all you need to do in 
order to enable OpenMP is pass -fopenmp flags during compiling and linking. The 
user should not need to know or care about how atomics are implemented.

It's not clear to me that our behavior for C++ atomics is good either.
From the documentation, it looks like the rationale is to avoid choosing 
between the GNU libatomic implementation and the compiler-rt implementation? We 
should probably make a default choice and provide a flag to override. That 
would seem more user-friendly to me.

 -Hal

>
> 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/G
>> nu.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?r
>> ev=336467&r1=336466&r2=336467&view=diff
>>
>> ===

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-19 Thread Hal Finkel via cfe-commits 

On 07/16/2018 01:19 PM, Jonas Hahnfeld wrote:
> [ 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.

Can you please elaborate on why you feel that this is problematic?

> 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.

From my perspective, because we instruct our users that all you need to
do in order to enable OpenMP is pass -fopenmp flags during compiling and
linking. The user should not need to know or care about how atomics are
implemented.

It's not clear to me that our behavior for C++ atomics is good either.
From the documentation, it looks like the rationale is to avoid choosing
between the GNU libatomic implementation and the compiler-rt
implementation? We should probably make a default choice and provide a
flag to override. That would seem more user-friendly to me.

 -Hal

>
> 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
>

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-16 Thread Jonas Hahnfeld via cfe-commits 
[ 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[[DE

r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

2018-07-06 Thread Alexey Bataev via cfe-commits 
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"