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

[SSCP] Add atomic support #941

Merged
merged 5 commits into from
Mar 20, 2023
Merged

[SSCP] Add atomic support #941

merged 5 commits into from
Mar 20, 2023

Conversation

illuhad
Copy link
Collaborator

@illuhad illuhad commented Feb 8, 2023

WIP. Adds support for atomics to the SSCP compiler.

  • Builtin infrastructure support
  • Implementation for llvm-to-amdgpu backend
  • Implementation for llvm-to-spirv backend
  • Implementation for llvm-to-ptx backend

@illuhad illuhad changed the title [SSCP][llvm-to-amdgpu] Add atomic support [SSCP] Add atomic support Feb 8, 2023
src/libkernel/sscp/amdgpu/atomic.cpp Outdated Show resolved Hide resolved
Comment on lines +350 to +351
// TODO: It seems that at least on gfx90a, there are additional unsafe atomic
// operations that can yield a performance improvement. Should we expose those?
Copy link
Collaborator

@sbalint98 sbalint98 Feb 13, 2023

Choose a reason for hiding this comment

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

For reference I think this GROAMCS issue is related. I think we should definitely expose them, however would it be possible to use the __builtin_amdgcn_ds_atomic_fadd_f32 https://gitlab.com/gromacs/gromacs/-/merge_requests/3493

Copy link
Collaborator Author

@illuhad illuhad Feb 13, 2023

Choose a reason for hiding this comment

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

Usually the set of enabled clang builtins depends on the frontend. It's likely that amdgcn builtins won't work by default with SSCP because the clang frontend does not know that it will run on anything else but the host CPU.

We could just create a simple __hip/opensycl_sscp_amdgcn_ds_atomic_fadd_f32() declaration and implement this inside amdgpu/atomic.cpp with a simple call to the amdgcn builtin. As long as this function is only invoked when JITing for amdgpu (and a GPU that supports it), it should be fine with SSCP.

Co-authored-by: sbalint98 <balint@streamhpc.com>
@illuhad illuhad marked this pull request as ready for review March 19, 2023 00:18
@illuhad
Copy link
Collaborator Author

illuhad commented Mar 19, 2023

This PR now provides complete implementations of atomic operations for SPIR-V, PTX, and amdgcn.

@illuhad
Copy link
Collaborator Author

illuhad commented Mar 20, 2023

Passes all current atomic tests on all backends. CC @RaulPPelaez

@illuhad illuhad merged commit 23323fc into develop Mar 20, 2023
@illuhad illuhad deleted the feature/sscp-atomic branch March 20, 2023 15:53
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.

None yet

2 participants