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 [email protected] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
