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
