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

Thu May 11 22:13:14 PDT 2023

tianshilei1992 added a comment.

In D149726#4336554 <https://reviews.llvm.org/D149726#4336554>, @MaskRay wrote:

> 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. 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.
>
> I don't understand this statement that requiring C++17 for `libomp` will definitely break a lot of users builds.
> Isn't that libomp is mostly built in C++17 modes, except in the `OPENMP_STANDALONE_BUILD` mode on some Windows configurations due to unrecognized `-std=c++17`?
>
> The `-std=c++17` occurrences in existing openmp/ files well suggest that requiring C++17 is intended (af28b27d31a5c13c31769c8551ffdcdc469fd5f4 <https://reviews.llvm.org/rGaf28b27d31a5c13c31769c8551ffdcdc469fd5f4> (2022-08)).
>
> Please be more specific what other configurations you'd like to support. If you want more builds supported I'd like to see a proposal for OpenMP like https://discourse.llvm.org/t/rfc-stand-alone-build-support/61291 (RFC: Stand-alone build support).
> Having no build bot for the configuration you care about and making a statement "it will break a lot of users builds" just creates a "haunted graveyard" <https://www.usenix.org/sites/default/files/conference/protected-files/srecon17americas_slides_reese.pdf> situation.

First, if you take a close look at the patch you pointed, it only checks the existence of the flag, but not enforces it. What's more, it is more for `libomptarget` instead of `libomp`.
Second, like I mentioned before, the development of `libomp` and `libomptarget` are diverged. We follow roughly the same standard as LLVM for `libomptarget`, but for `libomp` we choose to be much more conservative. Many HPC users only use `libomp`, but due to the complication of those HPC systems (for example, there are large number of HPC users still using GCC 4.8.5), enforcing C++17 globally will definitely break their build. I'm not against the change, but we have to discuss further in our meeting, especially with Intel folks since `libomp` was contributed by them.
As for build bot, we don't have build bot for `libomp` at all (we used to).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149726