KhronosGroup / SPIRV-Cross

SPIRV-Cross is a practical tool and library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages.
Apache License 2.0
2.02k stars 558 forks source link

[MSL] Image bindings wrong from GLSL->SPIRV-MSL #2363

Open preetishkakkar opened 1 month ago

preetishkakkar commented 1 month ago

Hi, I have below glsl code

layout (set = 0, binding = 0, r32ui) uniform coherent uimage2D my_image;

the generated MSL code is:

volatile device atomic_uint* my_image_atomic [[id(1)]]; 
texture2d<uint, access::write> my_image [[id(1)]]; 

This causes a compilation error note: attribute ‘id’ set location to 1, but minimum is 2, can you help?

Thanks.

HansKristian-Work commented 3 weeks ago

Do you have a complete GLSL or SPIR-V?

I cannot reproduce this with:

#version 450

layout(location = 0) out float FragColor;
layout(set = 0, binding = 0, r32ui) uniform coherent uimage2D img;

void main()
{
    imageAtomicAdd(img, ivec2(0), 5u);
}

SPIR-V:

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 23
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %main "main" %FragColor
               OpExecutionMode %main OriginUpperLeft
               OpSource GLSL 450
               OpName %main "main"
               OpName %img "img"
               OpName %FragColor "FragColor"
               OpDecorate %img DescriptorSet 0
               OpDecorate %img Binding 0
               OpDecorate %img Coherent
               OpDecorate %FragColor Location 0
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
          %7 = OpTypeImage %uint 2D 0 0 0 2 R32ui
%_ptr_UniformConstant_7 = OpTypePointer UniformConstant %7
        %img = OpVariable %_ptr_UniformConstant_7 UniformConstant
        %int = OpTypeInt 32 1
      %v2int = OpTypeVector %int 2
      %int_0 = OpConstant %int 0
         %13 = OpConstantComposite %v2int %int_0 %int_0
     %uint_5 = OpConstant %uint 5
     %uint_0 = OpConstant %uint 0
%_ptr_Image_uint = OpTypePointer Image %uint
     %uint_1 = OpConstant %uint 1
      %float = OpTypeFloat 32
%_ptr_Output_float = OpTypePointer Output %float
  %FragColor = OpVariable %_ptr_Output_float Output
       %main = OpFunction %void None %3
          %5 = OpLabel
         %17 = OpImageTexelPointer %_ptr_Image_uint %img %13 %uint_0
         %19 = OpAtomicIAdd %uint %17 %uint_1 %uint_0 %uint_5
               OpReturn
               OpFunctionEnd

MSL with ./spirv-cross --msl --msl-version 20000 /tmp/test.spv --msl-argument-buffers

#pragma clang diagnostic ignored "-Wmissing-prototypes"
#pragma clang diagnostic ignored "-Wunused-variable"

#include <metal_stdlib>
#include <simd/simd.h>
#include <metal_atomic>

using namespace metal;

// The required alignment of a linear texture of R32Uint format.
constant uint spvLinearTextureAlignmentOverride [[function_constant(65535)]];
constant uint spvLinearTextureAlignment = is_function_constant_defined(spvLinearTextureAlignmentOverride) ? spvLinearTextureAlignmentOverride : 4;
// Returns buffer coords corresponding to 2D texture coords for emulating 2D texture atomics
#define spvImage2DAtomicCoord(tc, tex) (((((tex).get_width() +  spvLinearTextureAlignment / 4 - 1) & ~( spvLinearTextureAlignment / 4 - 1)) * (tc).y) + (tc).x)

struct spvDescriptorSetBuffer0
{
    texture2d<uint> img [[id(0)]];
    volatile device atomic_uint* img_atomic [[id(1)]];
};

fragment void main0(constant spvDescriptorSetBuffer0& spvDescriptorSet0 [[buffer(0)]])
{
    uint _19 = atomic_fetch_add_explicit((volatile device atomic_uint*)&spvDescriptorSet0.img_atomic[spvImage2DAtomicCoord(int2(0), spvDescriptorSet0.img)], 5u, memory_order_relaxed);
}
HansKristian-Work commented 3 weeks ago

Also, this fallback for atomics is basically broken. If you target MSL 3.1, it should work better and use the native image atomic features.