[Openmp-commits] [PATCH] D149726: [OpenMP] Use CMAKE_CXX_STANDARD for setting the C++ version

[Martin Storsjö via Phabricator via Openmp-commits]

mstorsjo added a comment.

In D149726#4315555 <https://reviews.llvm.org/D149726#4315555>, @tianshilei1992 wrote:

> `libomp` is widely used and it only uses a small set of C++ features. Due to some historical issue that `libomptarget` and `libomp` are in the same `openmp` directory but their developments are quite diverged. I can see requiring C++17 for `libomp` will definitely break a lot user's build.

But overall this patch doesn’t change anything of that; if C++17 is supported, such a compiler flag is enabled, if not, the compiler default is used - this is the same behavior as before. The only difference is that it is done in a way that behaves consistently across compilers.

> That will need to be further discussed in our weekly multi-company meeting. The patch mentioned in the description didn't consider the special situation we have for `libomp`. If necessary, we might revert it in the future.

Sure, that might be for the best. But that patch has been in the tree for almost two months already, and if nobody else has run into issue with it, apparently it’s not an issue in practice?

In any case, I don’t have much of an opinion on that and don’t mind reverting it, but the fact that openmp is built in C++17 mode in some configs but not others, even where C++17 is supported, is a bit inconsistent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149726/new/

https://reviews.llvm.org/D149726