Skip to content

Conversation

@artv3
Copy link
Member

@artv3 artv3 commented Apr 24, 2023

Addressing missing docs in launch (#1068)

Attempt to address #1471

This PR adds in documentation for:
Dynamic shared memory usage
launch types, seq_launch_t, omp_launch_t, cuda_launch_t
loop tiling

@artv3 artv3 marked this pull request as draft April 25, 2023 16:42
@artv3 artv3 marked this pull request as ready for review April 25, 2023 21:52
@artv3 artv3 changed the title Draft: Launch docs updates Launch docs updates Apr 25, 2023
@artv3
Copy link
Member Author

artv3 commented Apr 25, 2023

@rhornung67 would it be possible to setup a read the docs for this branch?

@artv3 artv3 requested review from mdavis36, rchen20 and rhornung67 and removed request for rhornung67 May 17, 2023 18:34
@artv3
Copy link
Member Author

artv3 commented May 17, 2023

I've beefed up the launch docs in this PR, I believe it is ready for others to take a look at. CI is failing for CUDA for known reasons orthogonal to this PR.

@rhornung67
Copy link
Member

@artv3 I think the problem is that we're testing against camp main and @MrBurmark PR for camp was merged. That change doesn't export the CUDA targets properly.

@MrBurmark
Copy link
Member

Now that I know the right target to link against I might have a hope of updating camp to depend on that, though I don't know how to do anything more magical like blt might do.

@rhornung67
Copy link
Member

I can't find the message from David about what is the correct CMake call. I'm also looking into something else that may help.

@davidbeckingsale
Copy link
Member

I can't find the message from David about what is the correct CMake call. I'm also looking into something else that may help.

llnl/blt#585 (comment)

// kernel body using index 'i'
});
Copy link
Member

Choose a reason for hiding this comment

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

vertical alignment of }); with statement it corresponds to

Copy link
Member

@rhornung67 rhornung67 left a comment

Choose a reason for hiding this comment

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

Overall looks good. A few formatting text changes to be made noted in comments.

@artv3
Copy link
Member Author

artv3 commented Jun 13, 2023

@MrBurmark @rchen20 @mdavis36 did you guys want to take a look? or should I merge?

@artv3 artv3 merged commit 9ee289f into develop Jun 15, 2023
@artv3 artv3 deleted the artv3/launch-docs branch June 15, 2023 16:56
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.

6 participants