shader-slang / slang

Making it easier to work with shaders
MIT License
1.78k stars 159 forks source link

Support atomic intrinsics for Metal #4473

Closed jkwak-work closed 3 days ago

jkwak-work commented 4 days ago

This commit adds a support for the atomic intrinsics in Metal.

Metal requires the first argument for the atomic functions to be an atomic data type. This implementation rely on the fact that we can do a C-style type casting from a regular data type to an atomic data type.

csyonghe commented 3 days ago

Another case that has not been implemented here but we will need is to handle lowering of InterlockedAdd(texture[loc], Val).

Since metal does not support obtaining a pointer from a texture handle, we will need to extend our legalizeImageSubscript pass to translate InterlockedAdd(MetalAtomicCast(imageSubscript(texture,loc)), Val) into

call ".atomic_add" texture loc val 

We can recognize this case by checking the user of MetalAtomicCast, if the user is a call, and the callee has a [KnownBuiltin("metal_texture_atomic")] decoration (we need to mark the intrinsics in stdlib as such), then we should apply that transform in leglaizeImageSubscript.

jkwak-work commented 3 days ago

Another case that has not been implemented here but we will need is to handle lowering of InterlockedAdd(texture[loc], Val).

Since metal does not support obtaining a pointer from a texture handle, we will need to extend our legalizeImageSubscript pass to translate InterlockedAdd(MetalAtomicCast(imageSubscript(texture,loc)), Val) into

call ".atomic_add" texture loc val 

We can recognize this case by checking the user of MetalAtomicCast, if the user is a call, and the callee has a [KnownBuiltin("metal_texture_atomic")] decoration (we need to mark the intrinsics in stdlib as such), then we should apply that transform in leglaizeImageSubscript.

I was having this exact problem today. Thanks for the information.

I will follow the instructions tomorrow and see how it goes.

csyonghe commented 3 days ago

Ariel is also working on the legalizeImageSubscript pass recently so you may want to coordinate with him on this as well. But this PR can be merged as is without the texture support.