alpaka-group / alpaka

Abstraction Library for Parallel Kernel Acceleration :llama:
https://alpaka.readthedocs.io
Mozilla Public License 2.0
356 stars 74 forks source link

switch to C++20 only #2402

Closed psychocoderHPC closed 1 month ago

psychocoderHPC commented 1 month ago

Note: I have not touched the job generator rules. We can clean the rules later, the PR is already large and it will be hard to debug if a rule is wrong. Currently, if the CI is failing we know that it is a real issue not a combination filter rule problem.

SimeonEhrig commented 1 month ago

@psychocoderHPC It's really ironic, that you opened the PR ;-p I will check it later

psychocoderHPC commented 1 month ago

I was not able to remove boost::atomic_ref because it looks like libc++ required clang19 for this feature :-( https://en.cppreference.com/w/cpp/compiler_support

SimeonEhrig commented 1 month ago

I was not able to remove boost::atomic_ref because it looks like libc++ required clang19 for this feature :-( https://en.cppreference.com/w/cpp/compiler_support

Keep it out of this PR. We can discuss later, how to process with it. I think removing boost in general and make it optional for the use case of using libc++ <19 is a reasonable approach.

psychocoderHPC commented 1 month ago

I was not able to remove boost::atomic_ref because it looks like libc++ required clang19 for this feature :-( https://en.cppreference.com/w/cpp/compiler_support

Keep it out of this PR. We can discuss later, how to process with it. I think removing boost in general and make it optional for the use case of using libc++ <19 is a reasonable approach.

Yes did it already. I assumed std::atomic_ref is C++20 and we can simply boost::atomic_ref remove it but this was a wrong assumption.

SimeonEhrig commented 1 month ago

I was not able to remove boost::atomic_ref because it looks like libc++ required clang19 for this feature :-( https://en.cppreference.com/w/cpp/compiler_support

Keep it out of this PR. We can discuss later, how to process with it. I think removing boost in general and make it optional for the use case of using libc++ <19 is a reasonable approach.

Yes did it already. I assumed std::atomic_ref is C++20 and we can simply boost::atomic_ref remove it but this was a wrong assumption.

std::atomic_ref is C++ 20. The libc++ is just not C++ 20 feature complete.

SimeonEhrig commented 1 month ago

I think we have a bug in the CMake. Looks like the alpaka_CXX_STANDARD is not set everywhere. I used a C++20 in a following up PR and the alpakaAccTest.cpp is not compiled with --std=c++20 flag.

https://github.com/alpaka-group/alpaka/actions/runs/11113827449/job/30879030304?pr=2403

PR #2403 bases on this PR.

I should read warnings :-( It is compiled with C++20. The warning actual means, that consteval is not backwards compatible.

fwyzard commented 1 month ago

Where does -Wc++20-compat come from ?

psychocoderHPC commented 1 month ago

Where does -Wc++20-compat come from ?

Not sure, @SimeonEhrig points to another PR. This is nothing we set explicit in this PR. We use -Wno-c++98-compat in devCompileOptions maybe we need to set -Wc++20-compat and -Wc++20-compat-pedantic.

SimeonEhrig commented 1 month ago

Where does -Wc++20-compat come from ?

It's from the clang compiler and means you cannot compile the code with C++17 or older. https://clang.llvm.org/docs/DiagnosticsReference.html#wc-20-compat

Not sure, @SimeonEhrig points to another PR. This is nothing we set explicit in this PR. We use -Wno-c++98-compat in devCompileOptions maybe we need to set -Wc++20-compat and -Wc++20-compat-pedantic.

That's true. I thought it is a good idea to test this PR with real C++20 and not only compiling C++17 code with C++20 code. We had a similar bug in PR #2324 where we recognized, that our STL didn't support C++20 but we didn't care about until we had C++20 code.

But I think we can adding the flag -Wc++20-compat in the other PR. It is not required to compile with C++20 in general.

SimeonEhrig commented 1 month ago

@psychocoderHPC Did you removed the whole Clang-CUDA support?

psychocoderHPC commented 1 month ago

@psychocoderHPC Did you removed the whole Clang-CUDA support?

In the README CUDAclang has no tests because before all tests was executed with CUDA <12. This is something we can address in a separat PR. Maybe we test it in the CI and the readme is wrong or we have not activated testing for CUDA >=12 because of issues.

A quick check of the PIConGPU CI said that clang 18 should work with CUDA but we disabled it because of issues see #2387

SimeonEhrig commented 1 month ago

@psychocoderHPC Did you removed the whole Clang-CUDA support?

In the README CUDAclang has no tests because before all tests was executed with CUDA <12. This is something we can address in a separat PR. Maybe we test it in the CI and the readme is wrong or we have not activated testing for CUDA >=12 because of issues.

That's wrong. We test Clang 17 + CUDA 12.4: https://gitlab.com/hzdr/crp/alpaka/-/jobs/7964044232 Only debug builds for Clang 15 until 17 is forbidden:

https://github.com/alpaka-group/alpaka/blob/e20236d86be9a8093fa2efae44eaf2db79dc38db/script/job_generator/alpaka_filter.py#L19-L29

But we can move fixing the README.md to an extra PR.

SimeonEhrig commented 1 month ago

Merge, because the broken README.md should not block reducing CI jobs and enable new PRs. We fix it in another PR.