Interlocked Ops within member function causes invalid spirv generation

[Ipotrick]

For the following repro:

struct Buf
{
    uint t;

    [mutating]
    func tester()
    {
        uint prev;
        InterlockedAdd(t, 1, prev);
    }
};

struct Push
{
    Buf * b;
};

[[vk::push_constant]] Push push;

[shader("compute")]
[numthreads(1, 1, 1)]
void main()
{
    push.b->tester();
}

I get the following validation error for the generated spirv:

VUID-VkShaderModuleCreateInfo-pCode-08737(ERROR / SPEC): msgNum: -1520283006 - Validation Error: [ VUID-VkShaderModuleCreateInfo-pCode-08737 ] | MessageID = 0xa5625282 | vkCreateShaderModule(): pCreateInfo->pCode is not valid SPIR-V: [VUID-StandaloneSpirv-None-04686] AtomicIAdd: Vulkan spec only allows storage classes for atomic to be: Uniform, Workgroup, Image, StorageBuffer, PhysicalStorageBuffer or TaskPayloadWorkgroupEXT.
  %original = OpAtomicIAdd %uint %34 %uint_1 %uint_0 %uint_1
. The Vulkan spec states: If pCode is a pointer to SPIR-V code, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://vulkan.lunarg.com/doc/view/1.3.268.0/windows/1.3-extensions/vkspec.html#VUID-VkShaderModuleCreateInfo-pCode-08737)
    Objects: 0

This only happens when tester is a member function. As soon as tester is a free function taking a pointer to Buf this error vanishes.

Config:

Slang Version: v2024.1.10
pc platform: x86-64msvc windows11
slang::TargetDesc::format: SlangCompileTarget::SLANG_SPIRV
slang::TargetDesc::profile: glsl_450+spirv_1_4
slang::TargetDesc::flags: SLANG_TARGET_FLAG_GENERATE_SPIRV_DIRECTLY
slang::SessionDesc::defaultMatrixLayoutMode: SLANG_MATRIX_LAYOUT_COLUMN_MAJOR

[csyonghe]

This is because the this parameter of a member function is passed via inout convention. Which means the entire struct is read into thread local memory/register and passed in, and the object value after the call is written back to the original location. This means that this parameter will always be in threadlocal memory space.

We will need to add a diagnostic to prevent calling atomics like this.

[Ipotrick]

Ok. I think thats quite unfortunate, id prefer if this was a reference.

[csyonghe]

I agree with you that this is not ideal for this particular case. We can consider adding a [ref] attribute in addition to [mutating] which causes this to be passed by true reference. However there are still a lot of unanswered questions in language and IR design around true references and I don't think we are ready to roll out this feature yet.

[csyonghe]

I added [__ref] that you can use instead of [mutating] to make this passed by reference (currently implemented by inlining) instead of by inout.