GLSL: Consider that OpBufferPointerEXT can be tagged with RW flags.

Author: HansKristian-Work

reference/shaders-no-opt/asm/frag/descriptor-heap-nonwritable-nonreadable-alias.vk.nocompat.asm.spv16.frag.vk

@@ -0,0 +1,24 @@
+#version 460
+#extension GL_EXT_descriptor_heap : require
+#extension GL_EXT_nonuniform_qualifier : require
+#extension GL_EXT_shader_image_load_formatted : require
+
+layout(location = 0) out vec4 FragColor;
+layout(descriptor_heap, std430) readonly buffer SSBONoWrite
+{
+    vec4 data[];
+} spvSSBONoWriteResourceHeap[];
+
+layout(descriptor_heap, std430) writeonly buffer SSBONoRead
+{
+    vec4 data[];
+} spvSSBONoReadResourceHeap[];
+
+
+void main()
+{
+    FragColor = vec4(0.0);
+    spvSSBONoReadResourceHeap[2].data[0] = spvSSBONoWriteResourceHeap[2].data[2];
+    FragColor = spvSSBONoWriteResourceHeap[2].data[2];
+}
+

shaders-no-opt/asm/frag/descriptor-heap-nonwritable-nonreadable-alias.vk.nocompat.asm.spv16.frag

@@ -0,0 +1,65 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 11
+; Bound: 31
+; Schema: 0
+               OpCapability Shader
+               OpCapability UntypedPointersKHR
+               OpCapability DescriptorHeapEXT
+               OpExtension "SPV_EXT_descriptor_heap"
+               OpExtension "SPV_KHR_untyped_pointers"
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %main "main" %FragColor %resource_heap
+               OpExecutionMode %main OriginUpperLeft
+               OpSource GLSL 460
+               OpSourceExtension "GL_EXT_descriptor_heap"
+               OpSourceExtension "GL_EXT_nonuniform_qualifier"
+               OpName %main "main"
+               OpName %FragColor "FragColor"
+               OpName %resource_heap "resource_heap"
+               OpName %SSBO "SSBO"
+               OpMemberName %SSBO 0 "data"
+               OpDecorate %FragColor Location 0
+               OpDecorate %resource_heap BuiltIn ResourceHeapEXT
+               OpDecorate %_runtimearr_v4float ArrayStride 16
+               OpDecorate %SSBO Block
+			   OpDecorate %readonly NonWritable
+			   OpDecorate %writeonly NonReadable
+               OpMemberDecorate %SSBO 0 Offset 0
+               OpDecorateId %_runtimearr_20 ArrayStrideIdEXT %21
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+  %FragColor = OpVariable %_ptr_Output_v4float Output
+    %float_0 = OpConstant %float 0
+         %11 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%_ptr_UniformConstant = OpTypeUntypedPointerKHR UniformConstant
+%resource_heap = OpUntypedVariableKHR %_ptr_UniformConstant UniformConstant
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+      %int_2 = OpConstant %int 2
+%_runtimearr_v4float = OpTypeRuntimeArray %v4float
+       %SSBO = OpTypeStruct %_runtimearr_v4float
+%_ptr_StorageBuffer = OpTypeUntypedPointerKHR StorageBuffer
+%_ptr_StorageBuffer_SSBO = OpTypePointer StorageBuffer %SSBO
+%_ptr_StorageBuffer_v4float = OpTypePointer StorageBuffer %v4float
+         %20 = OpTypeBufferEXT StorageBuffer
+         %21 = OpConstantSizeOfEXT %int %20
+%_runtimearr_20 = OpTypeRuntimeArray %20
+       %uint = OpTypeInt 32 0
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+               OpStore %FragColor %11
+         %19 = OpUntypedAccessChainKHR %_ptr_UniformConstant %_runtimearr_20 %resource_heap %int_2
+         %readonly = OpBufferPointerEXT %_ptr_StorageBuffer_SSBO %19
+         %writeonly = OpBufferPointerEXT %_ptr_StorageBuffer_SSBO %19
+		 %chain_load = OpAccessChain %_ptr_StorageBuffer_v4float %readonly %int_0 %int_2
+		 %chain_store = OpAccessChain %_ptr_StorageBuffer_v4float %writeonly %int_0 %int_0
+		 %loaded = OpLoad %v4float %chain_load
+		 OpStore %chain_store %loaded
+               OpStore %FragColor %loaded
+               OpReturn
+               OpFunctionEnd

spirv_cross.cpp

@@ -5453,7 +5453,15 @@ void Compiler::analyze_descriptor_heap_types()
 				// BufferPointerEXT can return untyped or typed pointers.
 				// If it's typed, we resolve it here.
 				if (ptr_type.basetype == SPIRType::Struct)
-					add_unique_type(ptr_type.self, ptr_type.storage);
+				{
+					DescriptorHeapMeta meta = {};
+					meta.type = ptr_type.self;
+					meta.buffer_pointer_id = args[1];
+					meta.storage = ptr_type.storage;
+					meta.nonreadable = compiler.has_decoration(args[1], DecorationNonReadable);
+					meta.nonwritable = compiler.has_decoration(args[1], DecorationNonWritable);
+					add_unique_type(meta);
+				}
 				buffer_pointers[args[1]] = args[0];
 				break;
 			}
@@ -5523,7 +5531,9 @@ void Compiler::analyze_descriptor_heap_types()
 				         data_type.basetype == SPIRType::AccelerationStructure ||
 				         data_type.basetype == SPIRType::Sampler)
 				{
-					add_unique_type(data_type.self, StorageClassUniformConstant);
+					DescriptorHeapMeta meta = {};
+					meta.type = data_type.self;
+					add_unique_type(meta);
 				}
 				else if (buffer_pointers.count(args[3]) != 0)
 				{
@@ -5537,7 +5547,13 @@ void Compiler::analyze_descriptor_heap_types()
 					if (buffer_type.basetype == SPIRType::Void)
 					{
 						// This is where the pointer becomes typed, so register it here.
-						add_unique_type(data_type.self, buffer_type.storage);
+						DescriptorHeapMeta meta = {};
+						meta.type = data_type.self;
+						meta.buffer_pointer_id = args[3];
+						meta.storage = buffer_type.storage;
+						meta.nonreadable = compiler.has_decoration(args[3], DecorationNonReadable);
+						meta.nonwritable = compiler.has_decoration(args[3], DecorationNonWritable);
+						add_unique_type(meta);
 					}
 				}
 				break;
@@ -5552,18 +5568,24 @@ void Compiler::analyze_descriptor_heap_types()
 
 		explicit HeapHandler(Compiler &compiler_) : OpcodeHandler(compiler_) {}
 
-		std::vector<std::pair<uint32_t, StorageClass>> heap_types;
+		std::vector<DescriptorHeapMeta> heap_types;
 		std::unordered_map<uint32_t, TypeID> buffer_pointers;
 
-		void add_unique_type(uint32_t id, StorageClass storage)
+		void add_unique_type(const DescriptorHeapMeta &meta)
 		{
-			assert(id != 0);
+			assert(meta.type != 0);
 
 			for (auto &type : heap_types)
-				if (type.first == id)
+			{
+				if (type.type == meta.type && type.storage == meta.storage &&
+					type.buffer_pointer_id == meta.buffer_pointer_id &&
+					type.nonreadable == meta.nonreadable && type.nonwritable == meta.nonwritable)
+				{
 					return;
+				}
+			}
 
-			heap_types.emplace_back(id, storage);
+			heap_types.push_back(meta);
 		}
 	};
 

spirv_cross.hpp

@@ -1090,7 +1090,17 @@ class Compiler
 	SmallVector<uint32_t> physical_storage_non_block_pointer_types;
 	std::unordered_map<uint32_t, PhysicalBlockMeta> physical_storage_type_to_alignment;
 
-	std::vector<std::pair<uint32_t, StorageClass>> descriptor_heap_types;
+	struct DescriptorHeapMeta
+	{
+		TypeID type;
+
+		// For buffers
+		ID buffer_pointer_id;
+		StorageClass storage;
+		bool nonwritable;
+		bool nonreadable;
+	};
+	std::vector<DescriptorHeapMeta> descriptor_heap_types;
 	void analyze_descriptor_heap_types();
 
 	void analyze_variable_scope(SPIRFunction &function, AnalyzeVariableScopeAccessHandler &handler);

spirv_glsl.cpp

@@ -2595,14 +2595,44 @@ void CompilerGLSL::emit_buffer_reference_block(uint32_t type_id, bool forward_de
 	}
 }
 
-void CompilerGLSL::emit_buffer_block_native(const SPIRVariable *var, SPIRType *type, StorageClass storage)
+std::string CompilerGLSL::to_buffer_pointer_name_prefix(uint32_t ptr_id) const
 {
-	if (!type)
+	auto itr = std::find_if(descriptor_heap_types.begin(), descriptor_heap_types.end(),
+		[&](const DescriptorHeapMeta &meta) { return meta.buffer_pointer_id == ptr_id; });
+
+	assert(itr != descriptor_heap_types.end());
+
+	auto name = to_name(itr->type);
+
+	// The same block type can be instantiated with different read-write decorations.
+	if (itr->nonreadable)
+		name += "NoRead";
+	if (itr->nonwritable)
+		name += "NoWrite";
+
+	return join("spv", name);
+}
+
+void CompilerGLSL::emit_buffer_block_native(const SPIRVariable *var, const DescriptorHeapMeta *heap_meta)
+{
+	assert(var || heap_meta);
+
+	SPIRType *type;
+	if (var)
 		type = &get<SPIRType>(var->basetype);
+	else
+		type = &get<SPIRType>(heap_meta->type);
 
 	Bitset flags = var ? ir.get_buffer_block_flags(*var) : ir.get_buffer_block_type_flags(*type);
-	if (var)
-		storage = var->storage;
+	auto storage = var ? var->storage : heap_meta->storage;
+
+	if (heap_meta)
+	{
+		if (heap_meta->nonreadable)
+			flags.set(DecorationNonReadable);
+		if (heap_meta->nonwritable)
+			flags.set(DecorationNonWritable);
+	}
 
 	bool ssbo = storage == StorageClassStorageBuffer || storage == StorageClassShaderRecordBufferKHR ||
 	            has_decoration(type->self, DecorationBufferBlock);
@@ -2615,6 +2645,15 @@ void CompilerGLSL::emit_buffer_block_native(const SPIRVariable *var, SPIRType *t
 	// Block names should never alias, but from HLSL input they kind of can because block types are reused for UAVs ...
 	auto buffer_name = to_name(type->self, false);
 
+	if (heap_meta)
+	{
+		// The same block type can be instantiated with different read-write decorations.
+		if (heap_meta->nonreadable)
+			buffer_name += "NoRead";
+		if (heap_meta->nonwritable)
+			buffer_name += "NoWrite";
+	}
+
 	auto &block_namespace = ssbo ? block_ssbo_names : block_ubo_names;
 
 	// Shaders never use the block by interface name, so we don't
@@ -2694,7 +2733,15 @@ void CompilerGLSL::emit_buffer_block_native(const SPIRVariable *var, SPIRType *t
 	}
 	else
 	{
-		end_scope_decl(join("spv", to_name(type->self), "ResourceHeap[]"));
+		auto name = to_name(type->self);
+
+		// The same block type can be instantiated with different read-write decorations.
+		if (heap_meta->nonreadable)
+			name += "NoRead";
+		if (heap_meta->nonwritable)
+			name += "NoWrite";
+
+		end_scope_decl(join("spv", name, "ResourceHeap[]"));
 	}
 
 	statement("");
@@ -4147,7 +4194,7 @@ void CompilerGLSL::emit_resources()
 
 	for (const auto &heap_type : descriptor_heap_types)
 	{
-		auto &type = get<SPIRType>(heap_type.first);
+		auto &type = get<SPIRType>(heap_type.type);
 
 		if (type.basetype == SPIRType::Image || type.basetype == SPIRType::AccelerationStructure)
 		{
@@ -4167,7 +4214,7 @@ void CompilerGLSL::emit_resources()
 		}
 		else
 		{
-			emit_buffer_block_native(nullptr, &type, heap_type.second);
+			emit_buffer_block_native(nullptr, &heap_type);
 		}
 	}
 
@@ -12946,11 +12993,23 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 		if (untyped)
 		{
 			auto *var = maybe_get_backing_variable(ptr_id);
-			if (!var || !has_decoration(var->self, DecorationBuiltIn) ||
-				(BuiltIn(get_decoration(var->self, DecorationBuiltIn)) != BuiltInResourceHeapEXT &&
-				 BuiltIn(get_decoration(var->self, DecorationBuiltIn)) != BuiltInSamplerHeapEXT))
+
+			// Chase back to base SPIRExpression.
+			auto *expr = maybe_get<SPIRExpression>(ptr_id);
+			while (expr && !expr->buffer_pointer && expr->loaded_from)
+				expr = maybe_get<SPIRExpression>(expr->loaded_from);
+
+			// Buffer pointers stop the loaded from chain to deal with aliasing better, so carve that out specifically.
+			bool is_buffer_pointer = expr && expr->buffer_pointer;
+
+			if (!is_buffer_pointer)
 			{
-				SPIRV_CROSS_THROW("Untyped pointer access chains are currently only supported for descriptor heap access.");
+				if (!var || !has_decoration(var->self, DecorationBuiltIn) ||
+					(BuiltIn(get_decoration(var->self, DecorationBuiltIn)) != BuiltInResourceHeapEXT &&
+					 BuiltIn(get_decoration(var->self, DecorationBuiltIn)) != BuiltInSamplerHeapEXT))
+				{
+					SPIRV_CROSS_THROW("Untyped pointer access chains are currently only supported for descriptor heap access.");
+				}
 			}
 		}
 
@@ -12978,7 +13037,10 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 				// For further buffer access chains, we don't do any fixups since we have resolved to proper types.
 				// For buffer types we only prepend when the access chain starts from a BufferPointerEXT base.
 				// Multi-stage access chains are not possible for image types.
-				e = join("spv", to_name(data_type.self), e);
+				if (ptr_expr && ptr_expr->buffer_pointer)
+					e = join(to_buffer_pointer_name_prefix(ptr_expr->self), e);
+				else
+					e = join("spv", to_name(data_type.self), e);
 			}
 		}
 
@@ -13081,10 +13143,9 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 
 		if (untyped)
 		{
-			auto &data_type = get<SPIRType>(ops[2]);
 			auto *ptr_expr = maybe_get<SPIRExpression>(ptr_id);
 			if (ptr_expr && ptr_expr->buffer_pointer)
-				e = join("spv", to_name(data_type.self), e);
+				e = join(to_buffer_pointer_name_prefix(ptr_expr->self), e);
 		}
 
 		if (has_decoration(ptr_id, DecorationNonUniform))
@@ -13112,10 +13173,11 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 		// BufferPointerEXT can return a typed pointer, in which case we need to resolve the heap alias now.
 		auto &type = get<SPIRType>(type_id);
 		if (type.basetype == SPIRType::Struct)
-			e = join("spv", to_name(type.self), e);
+			e = join(to_buffer_pointer_name_prefix(result_id), e);
 
 		auto &expr = set<SPIRExpression>(result_id, std::move(e), type_id, true);
-		expr.loaded_from = chain_expr->loaded_from;
+		// There isn't any backing variable here. OpBufferPointerEXT is meant to be a memory declaration instruction.
+		expr.loaded_from = 0;
 		expr.access_chain = true;
 		expr.buffer_pointer = true;
 		expr.implied_read_expressions = chain_expr->implied_read_expressions;

spirv_glsl.hpp

@@ -678,8 +678,8 @@ class CompilerGLSL : public Compiler
 	void emit_extension_workarounds(ExecutionModel model);
 	void emit_subgroup_arithmetic_workaround(const std::string &func, Op op, GroupOperation group_op);
 	void emit_polyfills(uint32_t polyfills, bool relaxed);
-	void emit_buffer_block_native(const SPIRVariable *var, SPIRType *type,
-	                              StorageClass storage = StorageClassGeneric);
+	void emit_buffer_block_native(const SPIRVariable *var, const DescriptorHeapMeta *heap_meta = nullptr);
+	std::string to_buffer_pointer_name_prefix(uint32_t ptr_id) const;
 	void emit_buffer_reference_block(uint32_t type_id, bool forward_declaration);
 	void emit_buffer_block_legacy(const SPIRVariable &var);
 	void emit_buffer_block_flattened(const SPIRVariable &type);