Skip to content

Commit

Permalink
Merge pull request #2449 from KhronosGroup/glsl-atomic-fixes
Browse files Browse the repository at this point in the history
GLSL: Fix some issues with atomics
  • Loading branch information
HansKristian-Work authored Feb 18, 2025
2 parents 6a62df6 + b220e53 commit 2c32b6b
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#version 450
#extension GL_EXT_buffer_reference2 : require
#extension GL_EXT_buffer_reference_uvec2 : require
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(buffer_reference) buffer uintPointer;
layout(buffer_reference, buffer_reference_align = 4) buffer uintPointer
{
uint value;
};

layout(push_constant, std430) uniform Registers
{
uvec2 va;
} _6;

void main()
{
uint _24 = atomicMax(uintPointer(_6.va).value, 10u);
}

21 changes: 21 additions & 0 deletions reference/shaders-no-opt/comp/atomics-64bit.comp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#version 450
#if defined(GL_ARB_gpu_shader_int64)
#extension GL_ARB_gpu_shader_int64 : require
#elif defined(GL_NV_gpu_shader5)
#extension GL_NV_gpu_shader5 : require
#else
#error No extension available for 64-bit integers.
#endif
#extension GL_EXT_shader_atomic_int64 : require
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(binding = 0, std430) buffer SSBO
{
uint64_t v;
} _9;

void main()
{
uint64_t _18 = atomicMax(_9.v, 10ul);
}

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#error No extension available for 64-bit integers.
#endif
#extension GL_EXT_shader_image_int64 : require
#extension GL_EXT_shader_atomic_int64 : require
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(set = 0, binding = 0, r64ui) uniform u64image2D uimg;
Expand Down
43 changes: 43 additions & 0 deletions shaders-no-opt/asm/comp/atomic-on-bda-pod.asm.nocompat.vk.comp
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 25
; Schema: 0
OpCapability Shader
OpCapability PhysicalStorageBufferAddresses
OpExtension "SPV_KHR_physical_storage_buffer"
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpSource GLSL 450
OpSourceExtension "GL_EXT_buffer_reference"
OpSourceExtension "GL_EXT_buffer_reference_uvec2"
OpName %main "main"
OpName %Registers "Registers"
OpMemberName %Registers 0 "va"
OpName %_ ""
OpDecorate %Registers Block
OpMemberDecorate %Registers 0 Offset 0
%void = OpTypeVoid
%3 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%v2uint = OpTypeVector %uint 2
%Registers = OpTypeStruct %v2uint
%_ptr_PushConstant_Registers = OpTypePointer PushConstant %Registers
%_ = OpVariable %_ptr_PushConstant_Registers PushConstant
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%_ptr_PushConstant_v2uint = OpTypePointer PushConstant %v2uint
%_ptr_PhysicalStorageBuffer_uint = OpTypePointer PhysicalStorageBuffer %uint
%uint_10 = OpConstant %uint 10
%uint_1 = OpConstant %uint 1
%uint_0 = OpConstant %uint 0
%main = OpFunction %void None %3
%5 = OpLabel
%14 = OpAccessChain %_ptr_PushConstant_v2uint %_ %int_0
%15 = OpLoad %v2uint %14
%18 = OpBitcast %_ptr_PhysicalStorageBuffer_uint %15
%24 = OpAtomicUMax %uint %18 %uint_1 %uint_0 %uint_10
OpReturn
OpFunctionEnd
14 changes: 14 additions & 0 deletions shaders-no-opt/comp/atomics-64bit.comp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#version 450
#extension GL_EXT_shader_atomic_int64 : require
#extension GL_ARB_gpu_shader_int64 : require
layout(local_size_x = 1) in;

layout(set = 0, binding = 0) buffer SSBO
{
uint64_t v;
};

void main()
{
atomicMax(v, 10ul);
}
36 changes: 31 additions & 5 deletions spirv_glsl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5397,6 +5397,15 @@ string CompilerGLSL::to_non_uniform_aware_expression(uint32_t id)
return expr;
}

string CompilerGLSL::to_atomic_ptr_expression(uint32_t id)
{
string expr = to_non_uniform_aware_expression(id);
// If we have naked pointer to POD, we need to dereference to get the proper ".value" resolve.
if (should_dereference(id))
expr = dereference_expression(expression_type(id), expr);
return expr;
}

string CompilerGLSL::to_expression(uint32_t id, bool register_expression_read)
{
auto itr = invalid_expressions.find(id);
Expand Down Expand Up @@ -6999,9 +7008,12 @@ void CompilerGLSL::emit_atomic_func_op(uint32_t result_type, uint32_t result_id,
require_extension_internal("GL_EXT_shader_atomic_float");
}

if (type.basetype == SPIRType::UInt64 || type.basetype == SPIRType::Int64)
require_extension_internal("GL_EXT_shader_atomic_int64");

forced_temporaries.insert(result_id);
emit_op(result_type, result_id,
join(op, "(", to_non_uniform_aware_expression(op0), ", ",
join(op, "(", to_atomic_ptr_expression(op0), ", ",
to_unpacked_expression(op1), ")"), false);
flush_all_atomic_capable_variables();
}
Expand Down Expand Up @@ -13875,8 +13887,11 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
const char *increment = unsigned_type ? "0u" : "0";
emit_op(ops[0], ops[1],
join(op, "(",
to_non_uniform_aware_expression(ops[2]), ", ", increment, ")"), false);
to_atomic_ptr_expression(ops[2]), ", ", increment, ")"), false);
flush_all_atomic_capable_variables();

if (type.basetype == SPIRType::UInt64 || type.basetype == SPIRType::Int64)
require_extension_internal("GL_EXT_shader_atomic_int64");
break;
}

Expand All @@ -13888,8 +13903,12 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
// Ignore semantics for now, probably only relevant to CL.
uint32_t val = ops[3];
const char *op = check_atomic_image(ptr) ? "imageAtomicExchange" : "atomicExchange";
statement(op, "(", to_non_uniform_aware_expression(ptr), ", ", to_expression(val), ");");
statement(op, "(", to_atomic_ptr_expression(ptr), ", ", to_expression(val), ");");
flush_all_atomic_capable_variables();

auto &type = expression_type(ptr);
if (type.basetype == SPIRType::UInt64 || type.basetype == SPIRType::Int64)
require_extension_internal("GL_EXT_shader_atomic_int64");
break;
}

Expand Down Expand Up @@ -13924,7 +13943,10 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
increment = "-1";

emit_op(ops[0], ops[1],
join(op, "(", to_non_uniform_aware_expression(ops[2]), ", ", increment, ")"), false);
join(op, "(", to_atomic_ptr_expression(ops[2]), ", ", increment, ")"), false);

if (type.basetype == SPIRType::UInt64 || type.basetype == SPIRType::Int64)
require_extension_internal("GL_EXT_shader_atomic_int64");
}

flush_all_atomic_capable_variables();
Expand All @@ -13943,9 +13965,13 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
{
const char *op = check_atomic_image(ops[2]) ? "imageAtomicAdd" : "atomicAdd";
forced_temporaries.insert(ops[1]);
auto expr = join(op, "(", to_non_uniform_aware_expression(ops[2]), ", -", to_enclosed_expression(ops[5]), ")");
auto expr = join(op, "(", to_atomic_ptr_expression(ops[2]), ", -", to_enclosed_expression(ops[5]), ")");
emit_op(ops[0], ops[1], expr, should_forward(ops[2]) && should_forward(ops[5]));
flush_all_atomic_capable_variables();

auto &type = get<SPIRType>(ops[0]);
if (type.basetype == SPIRType::UInt64 || type.basetype == SPIRType::Int64)
require_extension_internal("GL_EXT_shader_atomic_int64");
break;
}

Expand Down
1 change: 1 addition & 0 deletions spirv_glsl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,7 @@ class CompilerGLSL : public Compiler
SPIRExpression &emit_uninitialized_temporary_expression(uint32_t type, uint32_t id);
void append_global_func_args(const SPIRFunction &func, uint32_t index, SmallVector<std::string> &arglist);
std::string to_non_uniform_aware_expression(uint32_t id);
std::string to_atomic_ptr_expression(uint32_t id);
std::string to_expression(uint32_t id, bool register_expression_read = true);
std::string to_composite_constructor_expression(const SPIRType &parent_type, uint32_t id, bool block_like_type);
std::string to_rerolled_array_expression(const SPIRType &parent_type, const std::string &expr, const SPIRType &type);
Expand Down

0 comments on commit 2c32b6b

Please sign in to comment.