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

[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