Add cuda::device::memcpy_async_tx by ahendriksen · Pull Request #405 · NVIDIA/cccl

ahendriksen · 2023-09-05T18:43:56Z

Description

Closes #404

Add cuda::device::memcpy_async_tx for global to shared memory loads.

Checklist

New or existing tests cover these changes.
The documentation is up to date with these changes.

gonzalobg

Initial review, this is probably blocked on the other PRs about arrive_tx and the TMA intrinsics.

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

gonzalobg · 2023-09-07T12:05:22Z

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

+Copies `size` bytes from global memory `src` to shared memory `dest` and arrives
+on a shared memory barrier `bar`, updating its transaction count by `size`
+bytes.
+


Docs should mention preconditions and effects.

I did the preconditions. I really don't grok what should be documented under effects, so I have left that as is. The current memcpy_async docs didn't have anything I could use as inspiration.

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

libcudacxx/include/cuda/barrier

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h

griwes

This seems fine. If we ever come up with the formulation for pipeline_tx we'll want to move the function into its own header and reuse much of the code between pipeline and barrier versions, but since that's very much a future thing, this is sufficient for now.

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

libcudacxx/include/cuda/std/detail/libcxx/include/barrier

miscco · 2023-09-08T06:54:40Z

libcudacxx/include/cuda/std/detail/libcxx/include/version

+// unconditionally. This permits checking for the feature in host code. When
+// __CUDA_MINIMUM_ARCH__ is available, we only enable the feature when the
+// hardware supports it.
+#if (!defined(__CUDA_MINIMUM_ARCH__)) || (defined(__CUDA_MINIMUM_ARCH__) && 900 <= __CUDA_MINIMUM_ARCH__)


question: @griwes @wmaxey Do we want to move such conditions into __config and give it a proper name like _LIBCUDACXX_REQUIRES_SM_90

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

ahendriksen

Thanks for the review. I have moved the docs, added tests, and improved the docs.

The only thing that I am still unsure of is the what the effects section should list. @gonzalobg

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

ahendriksen · 2023-09-19T15:04:37Z

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

+Copies `size` bytes from global memory `src` to shared memory `dest` and arrives
+on a shared memory barrier `bar`, updating its transaction count by `size`
+bytes.
+


I did the preconditions. I really don't grok what should be documented under effects, so I have left that as is. The current memcpy_async docs didn't have anything I could use as inspiration.

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md

libcudacxx/docs/extended_api/synchronization_primitives/barrier.md

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md

libcudacxx/.upstream-tests/test/cuda/memcpy_async/memcpy_async_tx.pass.cpp

ahendriksen

Thanks for the review. Suggestions have been implemented and branch has been rebased on top of main.

libcudacxx/.upstream-tests/test/cuda/memcpy_async/memcpy_async_tx.pass.cpp

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md

libcudacxx/.upstream-tests/test/cuda/memcpy_async/memcpy_async_tx.pass.cpp

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h

Co-authored-by: gonzalobg <65027571+gonzalobg@users.noreply.github.com>

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md

…ync_tx.md Co-authored-by: gonzalobg <65027571+gonzalobg@users.noreply.github.com>

miscco · 2023-10-13T15:46:33Z

Thanks a lot for adding this new feature 🎉

ahendriksen force-pushed the add-memcpy-async-tx branch from f400754 to 3398386 Compare September 6, 2023 10:09

gonzalobg requested changes Sep 7, 2023

View reviewed changes

griwes approved these changes Sep 8, 2023

View reviewed changes

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h Outdated Show resolved Hide resolved

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md Show resolved Hide resolved

miscco reviewed Sep 8, 2023

View reviewed changes

jrhemstad reviewed Sep 11, 2023

View reviewed changes

libcudacxx/include/cuda/std/detail/libcxx/include/__cuda/barrier.h Show resolved Hide resolved

jrhemstad reviewed Sep 11, 2023

View reviewed changes

libcudacxx/docs/extended_api/synchronization_primitives/barrier/memcpy_async_tx.md Show resolved Hide resolved

ahendriksen force-pushed the add-memcpy-async-tx branch from 3398386 to 9aab0cf Compare September 13, 2023 12:17

ahendriksen commented Sep 19, 2023

View reviewed changes

ahendriksen marked this pull request as ready for review September 20, 2023 07:58

ahendriksen requested review from a team as code owners September 20, 2023 07:58

ahendriksen requested review from ericniebler and wmaxey and removed request for a team September 20, 2023 07:58

ahendriksen force-pushed the add-memcpy-async-tx branch from 5ea0488 to 7ac7ce7 Compare September 20, 2023 08:04

gonzalobg requested changes Sep 26, 2023

View reviewed changes

ahendriksen commented Sep 29, 2023

View reviewed changes

ahendriksen force-pushed the add-memcpy-async-tx branch from 9f85f3c to abe3457 Compare September 29, 2023 10:13

miscco reviewed Sep 29, 2023

View reviewed changes

ahendriksen added 10 commits October 2, 2023 20:43

Implement cuda::device::memcpy_async_tx

71a6b29

Add documentation

c9ec019

Move docs

4b7d827

Update docs

2cc403b

Add test

014f83d

Uglify parameters

e0ca3f1

Fix code example

a75fa14

Assert preconditions

13e3c16

Fix memcpy_async_tx docs

c42c28c

Add static_assert to memcpy_async_tx

759132e

ahendriksen and others added 6 commits October 2, 2023 20:43

Add example link in barrier_arrive_tx docs

fc20864

Fix memcpy_async_tx test

f81c97e

Apply suggestions from code review

043e117

Co-authored-by: gonzalobg <65027571+gonzalobg@users.noreply.github.com>

Link to barrier_arrive_tx

760c592

Add _LIBCUDACXX_UNREACHABLE

1e52fdb

Use shared_memory_selector

b886685

ahendriksen force-pushed the add-memcpy-async-tx branch from abe3457 to b886685 Compare October 2, 2023 18:44

Merge branch 'main' into pr/ahendriksen/405

221a98f

miscco requested a review from gonzalobg October 12, 2023 06:42

gonzalobg reviewed Oct 12, 2023

View reviewed changes

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md Outdated Show resolved Hide resolved

gonzalobg reviewed Oct 12, 2023

View reviewed changes

libcudacxx/docs/extended_api/asynchronous_operations/memcpy_async_tx.md Outdated Show resolved Hide resolved

ahendriksen and others added 2 commits October 12, 2023 12:11

Update libcudacxx/docs/extended_api/asynchronous_operations/memcpy_as…

0de28d6

…ync_tx.md Co-authored-by: gonzalobg <65027571+gonzalobg@users.noreply.github.com>

Document void return type for consistency

c99ab07

miscco approved these changes Oct 12, 2023

View reviewed changes

miscco merged commit c7c46a3 into NVIDIA:main Oct 13, 2023

Conversation

ahendriksen commented Sep 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

gonzalobg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gonzalobg Sep 7, 2023

Choose a reason for hiding this comment

Uh oh!

ahendriksen Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

griwes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miscco Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ahendriksen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahendriksen Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahendriksen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miscco commented Oct 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

ahendriksen commented Sep 5, 2023 •

edited

Loading

ahendriksen left a comment •

edited

Loading