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

Fix kokkos hpx nvcc compilation #6492

Merged
merged 3 commits into from
May 20, 2024

Conversation

G-071
Copy link
Member

@G-071 G-071 commented May 19, 2024

With CUDA 12.3 nvcc does not outright crash anymore when compiling sender-receiver code and with #6452 the HPX sender-receiver examples compile and work again with nvcc after fixing some compilation errors.

The last remaining problem related to this was to compile the Kokkos HPX execution space (using sender-receivers internally) with nvcc: Here we still got a few compilation problems such as:

  • member_pack.hpp(143): error: function returning function is not allowed
  • split.hpp:289:38: error: use of deleted function ‘hpx::execution::experimental::detail::any_receiver<Ts>::any_receiver(const hpx::execution::experimental::detail::any_receiver<Ts>&) [with Ts = {}]’

This PR addresses these issues!

By itself, this PR is not quite enough to make the HPX execution space compile with nvcc, we still need one more change within Kokkos itself. However, this PR resolves all issues on the HPX side, the rest (some small template deduction stuff) will consequently be added as a Kokkos PR.

Pinging @msimberg as he came up with the solution, I was merely the one reporting the problem and testing the fixes!

G-071 added 2 commits May 19, 2024 09:19
This prevents the function returning a function nvcc compilation error
This prevents nvcc trying to copy the receiver instead
@G-071 G-071 requested a review from hkaiser as a code owner May 19, 2024 14:42
@StellarBot
Copy link

Performance test report

HPX Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
HPX Datetime2024-03-18T14:00:30+00:002024-05-19T14:42:12+00:00
HPX Commitd27ac2e77a5577
Compiler/opt/apps/llvm/13.0.1/bin/clang++ 13.0.1/opt/apps/llvm/13.0.1/bin/clang++ 13.0.1
Envfile
Clusternamerostamrostam
Hostnamemedusa08.rostam.cct.lsu.edumedusa08.rostam.cct.lsu.edu
Datetime2024-03-18T09:18:04.949759-05:002024-05-19T09:49:53.894718-05:00

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch(=)

Info

PropertyBeforeAfter
HPX Datetime2024-03-18T14:00:30+00:002024-05-19T14:42:12+00:00
HPX Commitd27ac2e77a5577
Compiler/opt/apps/llvm/13.0.1/bin/clang++ 13.0.1/opt/apps/llvm/13.0.1/bin/clang++ 13.0.1
Envfile
Clusternamerostamrostam
Hostnamemedusa08.rostam.cct.lsu.edumedusa08.rostam.cct.lsu.edu
Datetime2024-03-18T09:19:53.062988-05:002024-05-19T09:51:38.874577-05:00

Comparison

BENCHMARKFORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATORPARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATORSCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add(=)=(=)
Stream Benchmark - Scale(=)(=)-
Stream Benchmark - Triad(=)(=)(=)
Stream Benchmark - Copy(=)==

Info

PropertyBeforeAfter
HPX Datetime2024-03-18T14:00:30+00:002024-05-19T14:42:12+00:00
HPX Commitd27ac2e77a5577
Compiler/opt/apps/llvm/13.0.1/bin/clang++ 13.0.1/opt/apps/llvm/13.0.1/bin/clang++ 13.0.1
Envfile
Clusternamerostamrostam
Hostnamemedusa08.rostam.cct.lsu.edumedusa08.rostam.cct.lsu.edu
Datetime2024-03-18T09:20:13.002391-05:002024-05-19T09:51:55.378269-05:00

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (≤5%)
++/--Large performance improvement/degradation (≤10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

Copy link
Member

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.06% 50.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (0618b4a) 220261 187453 85.10%
Head commit (58ad3dd) 191067 (-29194) 162723 (-24730) 85.17% (+0.06%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#6492) 2 1 50.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy will stop sending the deprecated coverage status from June 5th, 2024. Learn more

@hkaiser hkaiser merged commit 404e3c8 into STEllAR-GROUP:master May 20, 2024
73 of 82 checks passed
@msimberg
Copy link
Contributor

Thanks @G-071! Just out of curiosity, could you check what your CMake configuration says about HPX_HAVE_BUILTIN_FORWARD_MOVE and HPX_HAVE_CXX_LAMBDA_CAPTURE_DECLTYPE, i.e. which version of HPX_FORWARD is your build using?

@G-071
Copy link
Member Author

G-071 commented May 22, 2024

Thanks @G-071! Just out of curiosity, could you check what your CMake configuration says about HPX_HAVE_BUILTIN_FORWARD_MOVE and HPX_HAVE_CXX_LAMBDA_CAPTURE_DECLTYPE, i.e. which version of HPX_FORWARD is your build using?

@msimberg I had deleted the build directory already, but upon rebuilding HPX and Kokkos the same way, the CMakeCache tells me No and Yes regarding those two:

HPX_WITH_BUILTIN_FORWARD_MOVE:INTERNAL=FALSE
//Result of TRY_COMPILE
HPX_WITH_BUILTIN_FORWARD_MOVE_RESULT:INTERNAL=FALSE

and

HPX_WITH_CXX_LAMBDA_CAPTURE_DECLTYPE:INTERNAL=TRUE
//Result of TRY_COMPILE
HPX_WITH_CXX_LAMBDA_CAPTURE_DECLTYPE_RESULT:INTERNAL=TRUE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants