RAJA icon indicating copy to clipboard operation
RAJA copied to clipboard
verified publisher icon LLNL

cmake: Move minimum cxx standard to 17

Open johnbowen42 opened this issue 6 months ago • 7 comments

This PR changes cmake to support a minimum cxx standard of 17.

Reviewers--should we just be moving straight to 20?

johnbowen42 avatar Jun 13 '25 19:06 johnbowen42

This PR changes cmake to support a minimum cxx standard of 17.

Reviewers--should we just be moving straight to 20?

I thought straight to 20 was the plan.

artv3 avatar Jun 13 '25 23:06 artv3

This PR changes cmake to support a minimum cxx standard of 17. Reviewers--should we just be moving straight to 20?

I thought straight to 20 was the plan.

We discussed in our last meeting. I was under the impression we needed to wait slightly longer. @rhornung67 can you confirm?

johnbowen42 avatar Jun 13 '25 23:06 johnbowen42

This PR changes cmake to support a minimum cxx standard of 17. Reviewers--should we just be moving straight to 20?

I thought straight to 20 was the plan.

We discussed in our last meeting. I was under the impression we needed to wait slightly longer. @rhornung67 can you confirm?

That sounds good, I missed the last meeting.

artv3 avatar Jun 13 '25 23:06 artv3

@johnbowen42 @artv3 let's make sure this passes all of our CI checks. @johnbowen42 I don't see the latest camp pulled in. Was that done somewhere else?

After this gets merged, we will work on updating our CI compilers and move to C++20. Does that plan work for you folks?

rhornung67 avatar Jun 16 '25 15:06 rhornung67

@rhornung67 I’d like to test this against #1858. To do so, I first need https://github.com/LLNL/radiuss-spack-configs/pull/133 merged in RSC@main.

adrienbernede avatar Jun 24 '25 18:06 adrienbernede

@rhornung67 I’d like to test this against #1858. To do so, I first need LLNL/radiuss-spack-configs#133 merged in RSC@main.

@adrienbernede are you asking me to approve something? It looks like what you need to be merged is approved and can be merged.

rhornung67 avatar Jun 24 '25 18:06 rhornung67

@rhornung67 I was simply reacting to your comment:

let's make sure this passes all of our CI checks

and laying down the path forward as far as I am concerned.

adrienbernede avatar Jun 25 '25 05:06 adrienbernede

  • [x] Merge change in RSC: https://github.com/LLNL/radiuss-spack-configs/pull/133
  • [x] Bump RSC to merge commit
  • [ ] Pass tests on tioga and tuolumne with rocm 6.4.1 (requires c++17 by default):
    • https://github.com/LLNL/RAJA/pull/1858
    • https://github.com/LLNL/RAJAPerf/pull/520 -> No need to wait for merge there, just using the tests as proof of c++17 support.

adrienbernede avatar Jul 02 '25 10:07 adrienbernede