shader: zero image descriptors that only unreached blocks use

src/graphics/shader/recompiler/ir/Serialize.h

@@ -165,7 +165,7 @@ KYTY_IO_FIELDS(DescriptorSource::IndirectDescriptor, a, b, c, d, e, f, g, h, i,
                q, r)
 KYTY_IO_FIELDS(DescriptorSource, a, b, c)
 KYTY_IO_FIELDS(SrtRead, a, b)
-KYTY_IO_FIELDS(ResourceBlock, a, b, c, d)
+KYTY_IO_FIELDS(ResourceBlock, a, b, c, d, e)
 KYTY_IO_FIELDS(UniformFill, a, b, c, d, e, f)
 KYTY_IO_FIELDS(UniformFillPlan, a, b, c, d)
 KYTY_IO_FIELDS(ResourceSpecialization::Buffer, a, b, c, d, e, f, g, h)

src/graphics/shader/recompiler/ir/ShaderIR.h

@@ -647,6 +647,7 @@ struct ResourceBlock {
 	Value                 condition;
 	std::vector<uint32_t> successors;
 	std::vector<uint32_t> sources;
+	std::vector<uint32_t> images;
 	std::vector<uint32_t> srt_reads;
 };
 

src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp

@@ -1220,14 +1220,16 @@ static std::vector<ResourceBlock> ResourceControlFlow(const Program& program, co
 				block.sources.push_back(program.info.buffers.at(memory.resource).source);
 			} else {
 				block.sources.push_back(program.info.images.at(memory.resource).source);
+				block.images.push_back(memory.resource);
 				if (image.needs_sampler) {
 					block.sources.push_back(program.info.samplers.at(memory.sampler).source);
 				}
 			}
 		}
-		std::ranges::sort(block.sources);
+		for (auto* list: {&block.sources, &block.images}) {
-		block.sources.erase(std::unique(block.sources.begin(), block.sources.end()),
+			std::ranges::sort(*list);
-		                    block.sources.end());
+			list->erase(std::unique(list->begin(), list->end()), list->end());
+		}
 	}
 	for (auto& block: blocks) block.condition = predicate(block.condition);
 	if (std::ranges::none_of(
@@ -1701,7 +1703,7 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime
 		}
 		if (source->indirect_descriptor.has_value()) {
 			snapshot.images[i] = {.dword_count = 8u};
-			if (!WalkedActive(program, image.source)) {
+			if (!WalkedActive(program, image.source) || !WalkedImageActive(program, i)) {
 				continue;
 			}
 			const auto& indirect = *source->indirect_descriptor;
@@ -1721,7 +1723,9 @@ bool MaterializeResources(const ResourcePlan& program, const SrtRuntime& runtime
 			if (!evaluate(image.source, snapshot.images[i])) {
 				return false;
 			}
-			if (!ValidImageDescriptor(snapshot.images[i], image.r128)) {
+			// An image listed only by unreached blocks reads zero, not its source's descriptor.
+			if (!WalkedImageActive(program, i) ||
+			    !ValidImageDescriptor(snapshot.images[i], image.r128)) {
 				snapshot.images[i].dwords.fill(0);
 			}
 		}

src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp

@@ -148,8 +148,8 @@ bool ImmediateBits(Value value, uint64_t& bits) {
 }
 
 // Scratch layout in bytes: per descriptor source its eight DWORDs and a known DWORD, one
-// activity byte per source, then one qword per walked value (all ones when unknown). The
+// activity byte per source, then per image, then one qword per walked value (all ones when
-// walker's value slots, captured addresses, reachability and states follow.
+// unknown). The walker's value slots, captured addresses, reachability and states follow.
 constexpr uint32_t DescriptorStride = 9u * sizeof(uint32_t);
 
 uint32_t ActiveOffset(const ResourcePlan& plan) {
@@ -157,7 +157,8 @@ uint32_t ActiveOffset(const ResourcePlan& plan) {
 }
 
 uint32_t OutputOffset(const ResourcePlan& plan) {
-	return (ActiveOffset(plan) + static_cast<uint32_t>(plan.descriptor_sources.size()) + 7u) & ~7u;
+	const auto active = plan.descriptor_sources.size() + plan.info.images.size();
+	return (ActiveOffset(plan) + static_cast<uint32_t>(active) + 7u) & ~7u;
 }
 
 const uint8_t* Scratch(const ResourcePlan& plan) {
@@ -1290,11 +1291,14 @@ void WalkerCompiler::Compile(std::span<const Value> values) {
 			for (const auto slot: blocks[block].srt_reads) flat(slot);
 			c.L(skip);
 		}
-		// A source no block lists is always active.
+		// A source or image no block lists is always active.
-		for (uint32_t source = 0; source < sources.size(); ++source) {
+		for (uint32_t index = 0; index < sources.size() + m_plan.info.images.size(); ++index) {
+			const bool     image = index >= sources.size();
+			const uint32_t id    = image ? index - static_cast<uint32_t>(sources.size()) : index;
 			bool listed = false;
 			for (uint32_t block = 0; block < blocks.size(); ++block) {
-				if (std::ranges::find(blocks[block].sources, source) == blocks[block].sources.end()) continue;
+				const auto& list = image ? blocks[block].images : blocks[block].sources;
+				if (std::ranges::find(list, id) == list.end()) continue;
 				if (listed) {
 					c.or_(c.al, c.byte[c.rdx + m_reach + block]);
 				} else {
@@ -1302,7 +1306,7 @@ void WalkerCompiler::Compile(std::span<const Value> values) {
 				}
 				listed = true;
 			}
-			const auto active = c.byte[c.rdx + ActiveOffset(m_plan) + source];
+			const auto active = c.byte[c.rdx + ActiveOffset(m_plan) + index];
 			if (listed) {
 				c.mov(active, c.al);
 			} else {
@@ -1409,6 +1413,10 @@ bool WalkedActive(const ResourcePlan& plan, uint32_t source) {
 	return Scratch(plan)[ActiveOffset(plan) + source] != 0u;
 }
 
+bool WalkedImageActive(const ResourcePlan& plan, uint32_t image) {
+	return Scratch(plan)[ActiveOffset(plan) + plan.descriptor_sources.size() + image] != 0u;
+}
+
 bool ValidateRuntimeValue(const ResourcePlan& program, Value value, RuntimeValueType type) {
 	return RuntimeValidator(program, type).Run(value);
 }

src/graphics/shader/recompiler/ir/passes/SrtWalker.h

@@ -78,6 +78,7 @@ bool WalkSrt(const ResourcePlan& plan, const SrtRuntime& runtime, std::vector<ui
 bool WalkedValue(const ResourcePlan& plan, Value value, uint32_t& result);
 bool WalkedDescriptor(const ResourcePlan& plan, uint32_t source, DescriptorValue& result);
 bool WalkedActive(const ResourcePlan& plan, uint32_t source);
+bool WalkedImageActive(const ResourcePlan& plan, uint32_t image);
 // Length of the guest load at rip if it belongs to the walker running on this thread, else 0.
 // The exception handler completes such a load as a zero read.
 uint32_t SrtWalkerLoadLength(uint64_t rip);

tests/ResourceMaterializationTests.cpp

@@ -1314,6 +1314,62 @@ void TestSignedStorageSpecialization() {
   }
 }
 
+// One descriptor serves a store, an atomic that a user-data condition guards, and an image no
+// block lists. Unreached, the atomic reads a zero descriptor instead of the RGBA8 format the
+// others use; reached, it rejects that format.
+void TestUnreachedImageIgnoresSharedDescriptor(bool atomic64) {
+  using namespace Libs::Graphics::ShaderRecompiler::IR;
+  using Libs::Graphics::ShaderRecompiler::CFG::TerminatorKind;
+  using Libs::Graphics::Prospero::BufferFormat;
+  Program program;
+  program.stage = Libs::Graphics::ShaderType::Compute;
+  program.srt_plan_complete = program.resource_tracking_complete = true;
+  program.user_data_count = 2u;
+  auto &entry = AddValueBlock(program);
+  auto &guarded = AddValueBlock(program);
+  auto &done = AddValueBlock(program);
+  const auto user = [&](uint32_t index) {
+    return Value(&entry.AppendNewInst(ValueOpcode::GetUserData, {Value(ScalarReg(index))}));
+  };
+  DescriptorSource source;
+  source.dword_count = 8u;
+  source.dwords.fill(Value(0u));
+  source.dwords[0] = Value(0x100u);
+  source.dwords[1] = user(0);
+  source.dwords[3] = Value(uint32_t(Libs::Graphics::Prospero::ImageType::kColor2D) << 28u);
+  program.descriptor_sources.push_back(source);
+  const auto dimension = Libs::Graphics::ShaderRecompiler::Decoder::ImageDimension::Dim2D;
+  for (const bool atomic : {false, true, false})
+    program.info.images.push_back({.source = 0u, .resource_class = ImageResourceClass::Storage,
+        .dimension = dimension, .written = true, .atomic = atomic, .atomic64 = atomic && atomic64});
+  for (uint32_t image = 0; image < 2u; ++image)
+    program.memory_info.push_back({.kind = ResourceKind::Image, .resource = image});
+  entry.AppendNewInst(ValueOpcode::ImageWrite, {Value(0u), Value(0u), Value(0u), Value(true)})
+      .SetFlags(MemoryFlags{.index = 0});
+  guarded.AppendNewInst(atomic64 ? ValueOpcode::ImageAtomicUMax64 : ValueOpcode::ImageAtomicUMax32,
+                        {Value(0u), Value(0u), Value(0u), Value(true)})
+      .SetFlags(MemoryFlags{.index = 1});
+  entry.condition = Value(&entry.AppendNewInst(ValueOpcode::INotEqual32, {user(1), Value(0u)}));
+  entry.terminator.kind = TerminatorKind::ConditionalBranch;
+  entry.terminator.true_block = &guarded;
+  entry.terminator.false_block = &done;
+  guarded.terminator.kind = TerminatorKind::Branch;
+  guarded.terminator.true_block = &done;
+  done.terminator.kind = TerminatorKind::Return;
+  auto plan = Extract(program);
+  std::array<uint32_t, 2> data{uint32_t(BufferFormat::k8_8_8_8UNorm) << 20u, 0u};
+  const SrtRuntime runtime{.user_data = data};
+  ResourceSnapshot snapshot;
+  ResourceSpecialization specialization;
+  Check(MaterializeResources(plan, runtime, snapshot, specialization) &&
+            snapshot.images[0].dwords[1] == data[0] && snapshot.images[2].dwords[1] == data[0] &&
+            snapshot.images[1] == DescriptorValue{.dword_count = 8u},
+        "unreached atomic image kept the shared descriptor or an unlisted image lost it");
+  data[1] = 1u;
+  Check(!MaterializeResources(plan, runtime, snapshot, specialization),
+        "reached atomic image accepted a format its width rejects");
+}
+
 void TestIndirectImageSamplerClasses(bool mixed) {
   using namespace Libs::Graphics::ShaderRecompiler::IR;
   using Libs::Graphics::Prospero::BufferFormat;
@@ -1451,6 +1507,7 @@ int main() {
   TestEmptyScalarBufferSpecialization();
   TestMixedSamplerVariantsShareRuntimeDescriptor();
   TestSignedStorageSpecialization();
+  for (const bool atomic64 : {false, true}) TestUnreachedImageIgnoresSharedDescriptor(atomic64);
   for (const bool mixed : {false, true}) TestIndirectImageSamplerClasses(mixed);
   TestShaderCacheRecords();
   std::puts("ResourceMaterializationTests: all cases passed");