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

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

Open
preetishkakkar opened this issue Aug 9, 2024 · 2 comments
Open

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

preetishkakkar opened this issue Aug 9, 2024 · 2 comments
Labels
question Further progress depends on answer from issue creator.

Comments

@preetishkakkar
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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.

@HansKristian-Work HansKristian-Work added the question Further progress depends on answer from issue creator. label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further progress depends on answer from issue creator.
Projects
None yet
Development

No branches or pull requests

2 participants