apt-sim/AdePT

C++ 17

hageboeck opened this issue · 7 comments

hageboeck commented

Would c++-17 be a thing? The benefit are more constexpr functions in the standard library, which can also be used on the GPU.

hahnjo commented

https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#constexpr-functions suggests:

By default, a constexpr function cannot be called from a function with incompatible execution space.

with the footnote:

The restrictions are the same as with a non-constexpr callee function.

So I don't really see a benefit. Yes, there's --expt-relaxed-constexpr but it's called "experimental" for a reason and may break at any moment's notice.

hageboeck commented

It works brilliantly, and nvidia seems to be moving towards more standard C++.

But maybe I should ask the question differently:
What is the reason not to move to c++17?

hahnjo commented

Maybe it helps to explain why you want to move to C++17. What are the constexpr functions that you want to use (with the experimental flag) on GPUs?

hageboeck commented

Let's put aside those functions for a second. I think when you start a project, you should take the latest stable c++ standard. As your project ages, the standard is falling behind pretty quickly.

And here a few functions:

  • array.size()
  • for (auto& elm : array)
  • Even nicer (but c++20, I didn't want to ask for too much)
void processStuff(std::span<data type> data) {
  for (auto& elm : data) {
  • auto& x, y, z = positionVector;
  • if constexpr
WitekPokorski commented

I would agree here with Stephan, moving to the latest standard can only help in the future (or even now).

sethrj commented

For what it's worth, a similar discussion about requiring/supporting expt-relaxed-constexpr by the Kokkos performance portability group is here: kokkos/kokkos#2411 . The consensus was not to default to using any of the --expt- flags because they are subject to instabilities.

One other issue to keep in mind is that C++17 support is only available in CUDA 11, so if you have any test or production systems still using CUDA 10 they will be unable to build with C++17 enabled. (When we started Celeritas, Summit only supported CUDA 10, so we decided to stay with C++14. With the recent upgrade defaulting to CUDA11 we might change that.)

WitekPokorski commented

We stick to C++14 + CUDA10 for the first prototype.