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 atomic intrinsics #308

Merged
merged 7 commits into from
May 18, 2022
Merged

Add atomic intrinsics #308

merged 7 commits into from
May 18, 2022

Conversation

tkf
Copy link
Contributor

@tkf tkf commented May 13, 2022

This patch introduces LLVM.Interop.atomic_pointer* on LLVMPtr mimicking Core.Intrinsics.atomic_pointer* API on Ptr. This is a superset of JuliaGPU/CUDA.jl#1393. It extracts the "CUDA-unaware" part of the atomics previously partially implemented in CUDA.jl. My motivation for this is to implement GPU-interop of https://github.com/JuliaConcurrent/UnsafeAtomics.jl and https://github.com/JuliaConcurrent/Atomix.jl without explicitly dispatching on CUDA.jl and AMDGPU.jl types.

As discussed in JuliaGPU/CUDA.jl#1393 the native code from this is not particularly good on CUDA. But that's probably fixable in LLVM (a long-term solution). As a short-term solution, I think CUDA.jl and AMDGPU.jl can use @device_override to override these intrinsics in their device context (e.g., use @llvm.nvvm.* intrinsics in CUDA).

cc @vchuravy @jpsamaroo

@vchuravy vchuravy requested a review from maleadt May 13, 2022 22:04
Copy link
Collaborator

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

This is complex enough that having it in one place seems sensible.

@vchuravy
Copy link
Collaborator

1.7 failed on the atomics test

Copy link
Collaborator

@maleadt maleadt 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 not really familiar with atomics and their APIs, but superficially this looks OK (minor some small changes).

CI sees to fail with assertions enabled though.

@tkf
Copy link
Contributor Author

tkf commented May 18, 2022

@maleadt I'm fixing bugs locally with LLVM assertion enabled. I'll ping you once I fix all bugs locally.

@tkf
Copy link
Contributor Author

tkf commented May 18, 2022

@maleadt OK, I think it's ready. Can you run the CI?

@maleadt maleadt merged commit 7304237 into JuliaLLVM:master May 18, 2022
@tkf tkf deleted the atomics branch May 18, 2022 10:17
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