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

Add helper class for thread and block identifiers #596

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

stephenswat
Copy link
Member

The number of parameters in a lot of our kernels and device functions is becoming absurdly large. Part of this is that we are passing variables like thread identifiers, block identifiers and block sizes for each of these functions. Not only does this make code harder to read, it may also increase register pressure on the device causing potentially slow spilling. This commit introduces a new type that handles thread identifiers more compactly and efficiently.

@stephenswat stephenswat added refactor Change the structure of the code cuda Changes related to CUDA hip Changes related to AMD HIP sycl Changes related to SYCL labels May 29, 2024
@stephenswat stephenswat requested a review from krasznaa May 29, 2024 11:38
@stephenswat
Copy link
Member Author

I haven't tested this on SYCL yet, I just guessed at the function names and crossed my fingers.

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

I'm on board with the general idea. In fact, I've wondered about something similar myself as well.

But let's do it in a cleaner (and I believe simpler) way.

@stephenswat stephenswat force-pushed the feat/thread_id branch 2 times, most recently from 912cda0 to d63e4c6 Compare May 29, 2024 12:06
@stephenswat
Copy link
Member Author

I've split this into a CUDA and a SYCL class now, unified by - you guessed it - a concept.

@stephenswat stephenswat requested a review from krasznaa June 24, 2024 14:49
@stephenswat stephenswat removed the hip Changes related to AMD HIP label Jun 24, 2024
@stephenswat stephenswat marked this pull request as draft June 26, 2024 10:01
@stephenswat stephenswat marked this pull request as ready for review July 4, 2024 14:38
@stephenswat stephenswat added the alpaka Changes related to Alpaka label Jul 5, 2024
@stephenswat
Copy link
Member Author

@CrossR could you check if the Alpaka code in this PR makes sense?

@CrossR
Copy link
Contributor

CrossR commented Jul 5, 2024

Looks good!

@stephenswat stephenswat requested a review from paulgessinger July 6, 2024 08:37
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

I'm happy with this code setup. Apart from the one comment, as long as it is brought up to date, I'm happy with this update to go in.

@stephenswat stephenswat force-pushed the feat/thread_id branch 5 times, most recently from a0f6c04 to 69c0adc Compare August 1, 2024 11:25
The number of parameters in a lot of our kernels and device functions is
becoming absurdly large. Part of this is that we are passing variables
like thread identifiers, block identifiers and block sizes for each of
these functions. Not only does this make code harder to read, it may
also increase register pressure on the device causing potentially slow
spilling. This commit introduces a new type that handles thread
identifiers more compactly and efficiently.
@stephenswat stephenswat merged commit 532d885 into acts-project:main Aug 1, 2024
20 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpaka Changes related to Alpaka cuda Changes related to CUDA refactor Change the structure of the code sycl Changes related to SYCL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants