shader-slang / slang

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

Metal: atomic instructions for buffers #4481

Open jkwak-work opened 3 days ago

jkwak-work commented 3 days ago

Problem description Most of the atomic instructions are implemented for Metal by PR #4473. But the atomic member functions for buffers are not implemented.

The current implementation piggy-back on how Spir-v does, but it doesn't actually work.

struct RWByteAddressBuffer
{
    void InterlockedAdd(UINT dest, UINT value)
    {
        __target_switch
        {
        case metal:
        case spirv:
            let buf = __getEquivalentStructuredBuffer<uint>(this); // <=== DON'T WORK FOR METAL
            ::InterlockedAdd(buf[dest / 4], value);
        }
    }
}

Goal We need to make the atomic member functions working for the buffers.

Repro steps We have atomic test for buffer for HLSL, "tests/hlsl-intrinsic/byte-address-buffer-atomics.slang". It can be copied over to "tests/metal/atomic-buffer.slang" and make it working.

But for a simple repro, the following code can be used for a test.

//TEST:SIMPLE: -entry computeMain -stage compute -target metal
//TEST:SIMPLE: -entry computeMain -stage compute -target metallib

RWByteAddressBuffer bbuffer;

[numthreads(1, 1, 1)]
void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
{
    bbuffer.InterlockedAdd(0, 1);
}
jkwak-work commented 2 days ago

The problem seems to be on the fact that Metal doesn't allow us to obtain a memory pointer access to the buffer. https://github.com/shader-slang/slang/pull/4473#issuecomment-2190388723

A proper solution would be to extend our legalizeImageSubscript().