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

[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