LLNL / RAJA

RAJA Performance Portability Layer (C++)
BSD 3-Clause "New" or "Revised" License
484 stars 102 forks source link

Update desul and assess atomics #1062

Open rhornung67 opened 3 years ago

rhornung67 commented 3 years ago

This should be done after https://github.com/LLNL/RAJA/issues/1515

rhornung67 commented 1 year ago

RAJA API does not expose scope or memory order features.

Notion of "sequential" atomic now exists.

Discuss how to stage deployment/API changes to users.

rchen20 commented 9 months ago

@rhornung67 @trws The latest Desul+BLT+RAJA+Camp combination works on RZANSEL when OpenMP=Off (and still works everywhere else when OpenMP=On, i.e. EAS3, TOSS4 Intel).

Notes for future Desul debugging:

trws commented 9 months ago

Is this with or without the patch to raja usage of desul that properly propagates its OpenMP flag? #1563

rchen20 commented 9 months ago

Is this with or without the patch to raja usage of desul that properly propagates its OpenMP flag? #1563

@trws This is without #1563, do we want to merge that into raja/develop?

rhornung67 commented 9 months ago

@rchen20 we do not want to merge that PR into develop. It contains hacks in desul, BLT, and RAJA I believe that we do not want as well as changes to GitLab CI configuration that we do not want. I think we only want the OpenMP flag thing that @trws asked about.

rchen20 commented 9 months ago

Ah I see, we just want the change to RAJA's CMakeLists.txt. Let me give that a try.

trws commented 9 months ago

Yeah, it should just be that. All the other ones are ensuring it got the newer blt version.

rchen20 commented 9 months ago

Yeah, it should just be that. All the other ones are ensuring it got the newer blt version.

@trws It worked after I also changed some of the Desul CMake. Here's a rough sketch of what I changed:

The latter 2 Desul modifications are mostly to ensure that the OpenMP flag is recognized and properly set, and I excluded the changes to desul/cmake/blt_boilerplate.cmake. Do you want me to create PRs in RAJA and Desul for this fix? I feel like I may have missed something, or created some extra CMake flags.

trws commented 9 months ago

Thanks for that reminder @rchen20, I left that one sitting when we were waiting on the BLT update. I'll ask Damien about getting a review so we can move that through. If there's something that should be fixed in 117, make a PR to that branch or post the modifications there and I'll add them in no problem. Thanks for chasing this down.