Per discussion in https://reviews.llvm.org/D74145, switch default C++ standard to C++ 14
Details
Diff Detail
Event Timeline
This should have a RFC on openmp-dev. As far as I understand the linked patch, the aim is to use std::make_unique which is just syntactic sugar IMO. Unless there are good reasons to switch, I'd object because it requires newer versions of compilers than many HPC systems have.
openmp/cmake/HandleOpenMPOptions.cmake | ||
---|---|---|
31–34 | You should rename this, otherwise an incremental build won't test the new flag IIRC. |
Unless there are good reasons to switch, I'd object because it requires newer versions of compilers than many HPC systems have.
I disagree. The use case we would be supporting is:
- building llvm libomptarget from source
- using it with a binary llvm dist, or some other toolchain
- won't download or build a newer toolchain
The advantage isn't in make_unique, it's in matching the rest of llvm which has required c++14 for months. It's not worth rehashing the stability-vs-progress discussion here when the parent project has already reached consensus.
You can do all that if the runtime only requires a subset of its parent project.
The advantage isn't in make_unique, it's in matching the rest of llvm which has required c++14 for months. It's not worth rehashing the stability-vs-progress discussion here when the parent project has already reached consensus.
I really hope that the OpenMP runtime has a broader usage than just with "the rest of llvm". It can be used standalone with Intel Compilers and GCC, and this is a great advantage for research.
Yes, evidently. It's catering to a user who wants to build libomptarget from C++11 source, but is not willing to build a compiler from C++11 source, and is not willing to download a binary version of such a compiler. That's surely the empty set.
I really hope that the OpenMP runtime has a broader usage than just with "the rest of llvm". It can be used standalone with Intel Compilers and GCC, and this is a great advantage for research.
I believe it does have such usage. I just don't believe that requiring the same compiler capability as the rest of llvm will hinder this, and I definitely believe in the ongoing inefficiency induced by sticking with an older C++ revision. Exactly the same arguments that (eventually) moved the rest of llvm up to C++14 apply here.
FWIW, I'm in strong favor of this, e.g., I suggested it in the other patch.
@Hahnfeld I do appreciate your concerns. Since you asked for an RFC, I wrote one: http://lists.llvm.org/pipermail/openmp-dev/2020-February/003176.html Let's move the discussion to the list and accept/reject this patch after we came to some kind of conclusion there.
openmp/cmake/HandleOpenMPOptions.cmake | ||
---|---|---|
31–34 | Oh, exactly, sorry for missing that. |
Issue https://github.com/llvm/llvm-project/issues/159 suggests we missed a file here. Perhaps plugins was previously relying on the compiler default?
You should rename this, otherwise an incremental build won't test the new flag IIRC.