Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++17 parallelism #273

Open
wants to merge 182 commits into
base: develop
Choose a base branch
from
Open

C++17 parallelism #273

wants to merge 182 commits into from

Conversation

jeffhammond
Copy link

I implemented C++17 parallel - std::for_each_n and friends - and have tested its correct function with GCC 10+, Intel 2021+ and NVHPC 22+.

There are a few issues I still need to document, which I will add next week.

@rhornung67 rhornung67 requested a review from artv3 November 4, 2022 18:22
@jeffhammond
Copy link
Author

jeffhammond commented Mar 28, 2023

Sorry, I was working on other things.

This is compiling with Intel 2023.0, Clang 14.0, GCC 11.3 and NVC++ 22.11 at least, although there are a few bugs.

There is a NVC++ compiler bug that affects Lambda_Seq MAT_MAT_SHARED, so I didn't bother with the StdPar ports. I don't really care about this one, because people should not implement matrix-matrix multiplication in RAJA, but use a library instead.

-------------------------------------------------------
Basic_MAT_MAT_SHARED
........................................................
Base_Seq-default           359.17234579009692780       0.0000000000000000000
Lambda_Seq-default         357.98891466713738432       1.1834311229595434745
RAJA_Seq-default           359.17234579009692780       0.0000000000000000000

This one fails because I wrote bad C++ code. It's fine with directive atomics.

-------------------------------------------------------
Basic_PI_ATOMIC
........................................................
Base_Seq-default           3.1415926535981304468       0.0000000000000000000
Lambda_Seq-default         3.1415926535981668621       -3.6415315207705134526e-14
RAJA_Seq-default           3.1415926535981668621       -3.6415315207705134526e-14
Base_StdPar-default        0.00010767665459625632687   3.1414849769435341906
Lambda_StdPar-default      0.00011083609681998327187   3.1414818175013104636

Intel fails here. I suspect it's a compiler bug, but I have not proven this.

-------------------------------------------------------
Apps_DIFFUSION3DPA
........................................................
Base_Seq-default           153401087.86143057652       0.0000000000000000000
RAJA_Seq-default           153401087.86143057652       0.0000000000000000000
Base_StdPar-default        1380609790.7528751888       -1227208702.8914446123

Clang and GCC fail to link on x86 Linux because of TBB problems. On MacOS (Apple Silicon), GCC 12 builds and runs fine, albeit single-threaded.

Here is how you reproduce things:

NVHPC CPU

mkdir -p cpu-stdpar && cd cpu-stdpar && git clean -dfx ; cmake .. -DCMAKE_C_COMPILER=nvc -DCMAKE_CXX_COMPILER=nvc++ -DCMAKE_CXX_FLAGS="-std=c++20 -stdpar=multicore" -DENABLE_STDPAR=1 && make -j`nproc` && ./bin/raja-perf.exe -sp 

NVHPC GPU

mkdir -p gpu-stdpar && cd gpu-stdpar && git clean -dfx ; cmake .. -DCMAKE_C_COMPILER=nvc -DCMAKE_CXX_COMPILER=nvc++ -DCMAKE_CXX_FLAGS="-std=c++20 -stdpar=gpu" -DENABLE_STDPAR=1 && make -j`nproc` && ./bin/raja-perf.exe -sp

GCC

mkdir -p gcc && cd gcc && git clean -dfx ; cmake .. -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_FLAGS="-std=c++20" -DENABLE_STDPAR=1 && make -j`nproc` && ./bin/raja-perf.exe -sp

Intel

mkdir -p intel && cd intel && git clean -dfx ; cmake .. -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx -DCMAKE_CXX_FLAGS="-std=c++20 -qtbb" -DENABLE_STDPAR=1 && make -j`nproc` && ./bin/raja-perf.exe -sp

@jeffhammond jeffhammond marked this pull request as ready for review March 28, 2023 10:15
@jeffhammond
Copy link
Author

@trws I recall you were interested in this.

@jeffhammond
Copy link
Author

I won't claim this is perfect yet, but I've already had to redo the entire commit process because of git submodule conflicts, and generally git does a terrible job with merges whenever upstream moves, so it would be super awesome if we could get it good enough to merge and then add the rest of the polish.

At this point, I've spent more time dealing with git merge problems than writing the code...

@jeffhammond
Copy link
Author

And please squash when you merge (assuming you are willing), because the git history here is useless and embarrassing.

@trws
Copy link
Member

trws commented Mar 31, 2023

Thanks for taking the time to put this together @jeffhammond! Squash shouldn't be a problem, but it looks like right now it's missing an ifdef or cmake conditional or similar because the stdpar utils are getting pulled into builds for unsupported compilers and taking out the other azure builds. @rhornung67, if the other builds get fixed up, think we can pull this in?

@jeffhammond
Copy link
Author

That header is only included in StdPar files but it seems that ENABLE_STDPAR is somehow on by default, which is odd, because I added ENABLE_STDPAR for that, in the same manner as ENABLE_KOKKOS, which is not on by default.

I guess I need to put #if defined(RUN_STDPAR) or #if defined(RAJA_ENABLE_STDPAR) guard in every source file.

% git grep StdParUtils | grep -v "StdPar\.cpp" | wc -l
       0

https://dev.azure.com/llnl/RAJAPerf/_build/results?buildId=4287&view=logs&j=1d71948b-e22b-5924-c187-f5ea65f2dc94&t=0aae7148-a33e-5eb0-43c7-9fd4bde805b2&l=1540

#8 17.20 In file included from /home/raja/workspace/src/apps/CONVECTION3DPA-StdPar.cpp:13:0:
#8 17.20 /home/raja/workspace/src/./common/StdParUtils.hpp:56:10: fatal error: execution: No such file or directory
#8 17.20  #include <execution>
#8 17.20           ^~~~~~~~~~~
#8 17.20 compilation terminated.

@jeffhammond
Copy link
Author

i have implemented guards everywhere so this change should be a no-op on your CI. if you want to test StdPar, please set -DENABLE_STDPAR=1 with supported compilers.

@trws
Copy link
Member

trws commented Mar 31, 2023

It looks like almost everything builds, except this:

##[error]:105:2: error: JEFF
##[error]#8 268.1 #error JEFF
##[error]#8 268.1  ^
##[error] error generated.
##[error]#8 268.7 make[2]: *** [src/stream/CMakeFiles/stream.dir/build.make:104: src/stream/CMakeFiles/stream.dir/ADD-StdPar.cpp.o] Error 1

@jeffhammond
Copy link
Author

Some dumb guy committed something he should not have but it's not clear who it was 🤦‍♂️

I'll fix it asap.

Remove debug print preprocessing
@jeffhammond jeffhammond force-pushed the new-stdpar branch 2 times, most recently from cd48d2a to 38bf9bd Compare April 17, 2023 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants