graphics: do not arm a fast clear from a read-modify-write dispatch (#428)

* graphics: do not arm a fast clear from a read-modify-write dispatch

* tests: cover compute metadata clear classification

---------

Co-authored-by: nmzik <Nmzik@mail.ru>

CMakeLists.txt

@@ -574,6 +574,8 @@ if(BUILD_TESTING)
 		COMMAND $<TARGET_FILE:shader_recompiler_compute_tests> --storage-sampled-only)
 	add_test(NAME texture_cache_depth_readback
 		COMMAND $<TARGET_FILE:shader_recompiler_compute_tests> --depth-readback-only)
+	add_test(NAME compute_meta_clear_classification
+		COMMAND $<TARGET_FILE:shader_recompiler_compute_tests> --compute-meta-clear-only)
 	add_test(NAME buffer_cache_dirty_gc
 		COMMAND $<TARGET_FILE:shader_recompiler_compute_tests> --buffer-cache-gc-only)
 

src/graphics/guest_gpu/command_processor/pm4Handlers.cpp

@@ -3635,6 +3635,9 @@ void GraphicsInitJmpTablesShIndirect() {
 	g_hw_sh_indirect_func[Pm4::SPI_SHADER_PACE_ID_GS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
 		HwShIgnoreShaderRegister(cmd_offset, value);
 	};
+	g_hw_sh_indirect_func[Pm4::SPI_SHADER_PGM_RSRC4_GS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
+		HwShIgnoreShaderRegister(cmd_offset, value);
+	};
 	g_hw_sh_indirect_func[Pm4::SPI_GRAPHICS_SHADER_CONTROL_GS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
 		HwShIgnoreShaderRegister(cmd_offset, value);
 	};
@@ -3644,6 +3647,12 @@ void GraphicsInitJmpTablesShIndirect() {
 	g_hw_sh_indirect_func[Pm4::SPI_SHADER_USER_DATA_ADDR_HI_GS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
 		HwShIgnoreShaderRegister(cmd_offset, value);
 	};
+	g_hw_sh_indirect_func[Pm4::SPI_SHADER_PGM_CHKSUM_HS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
+		HwShIgnoreShaderRegister(cmd_offset, value);
+	};
+	g_hw_sh_indirect_func[Pm4::SPI_SHADER_PGM_RSRC4_HS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
+		HwShIgnoreShaderRegister(cmd_offset, value);
+	};
 	g_hw_sh_indirect_func[Pm4::SPI_GRAPHICS_SHADER_CONTROL_HS] = [](KYTY_HW_SH_INDIRECT_ARGS) {
 		HwShIgnoreShaderRegister(cmd_offset, value);
 	};
@@ -3818,6 +3827,11 @@ void GraphicsInitJmpTablesUcIndirect() {
 		r.vertex_group_size    = KYTY_PM4_GET(value, GE_CNTL, VERT_GRP_SIZE);
 		cp.GetUcfg().SetGeControl(r);
 	};
+	g_hw_uc_indirect_func[Pm4::UC_PARAMETER_OVERSUBSCRIPTION] = [](KYTY_HW_UC_INDIRECT_ARGS) {
+		(void)cp;
+		(void)cmd_offset;
+		(void)value;
+	};
 	g_hw_uc_indirect_func[Pm4::GE_USER_VGPR_EN] = [](KYTY_HW_UC_INDIRECT_ARGS) {
 		HW::GeUserVgprEn r;
 		r.vgpr1 = KYTY_PM4_GET(value, GE_USER_VGPR_EN, EN_USER_VGPR1) != 0;

src/graphics/guest_gpu/pm4.h

@@ -834,8 +834,9 @@ constexpr uint32_t SPI_SHADER_USER_ACCUM_ESGS_0                   = 0xB2;
 constexpr uint32_t SPI_SHADER_USER_ACCUM_ESGS_3                   = 0xB5;
 constexpr uint32_t SPI_SHADER_PGM_LO_ES                           = 0xC8;
 constexpr uint32_t SPI_SHADER_PGM_HI_ES                           = 0xC9;
-// Retained only as an opaque AGC compiler-record tag; 0x100 is not a PS5 SH register.
+// Private AGC registers used by shader binaries submitted through SET_SH_REG_INDIRECT.
 constexpr uint32_t SPI_SHADER_PGM_CHKSUM_HS                       = 0x100;
+constexpr uint32_t SPI_SHADER_PGM_RSRC4_HS                        = 0x101;
 constexpr uint32_t SPI_SHADER_USER_DATA_ADDR_LO_HS                = 0x102;
 constexpr uint32_t SPI_SHADER_USER_DATA_ADDR_HI_HS                = 0x103;
 constexpr uint32_t SPI_GRAPHICS_SHADER_CONTROL_HS                 = 0x107;
@@ -976,7 +977,7 @@ constexpr uint32_t GE_USER_VGPR1  = 0x25C;
 constexpr uint32_t GE_USER_VGPR2  = 0x25D;
 constexpr uint32_t GE_USER_VGPR3  = 0x25E;
 constexpr uint32_t GE_STEREO_CNTL = 0x25F;
-constexpr uint32_t GE_PC_ALLOC    = 0x260;
+constexpr uint32_t UC_PARAMETER_OVERSUBSCRIPTION = 0x260;
 
 constexpr uint32_t GE_USER_VGPR_EN                     = 0x262;
 constexpr uint32_t GE_USER_VGPR_EN_EN_USER_VGPR1_SHIFT = 0;

src/graphics/host_gpu/renderer/renderCompute.cpp

@@ -54,7 +54,9 @@ bool RenderExecutor::TryConsumeComputeMetaClear(const ShaderComputeInputInfo& in
 	for (uint32_t i = 0; i < program.info.buffers.size(); i++) {
 		const auto& resource   = program.info.buffers[i];
 		const auto  descriptor = DecodeNativeDescriptor<ShaderBufferResource>(resources.buffers[i]);
-		if (!resource.written && cache.IsMeta(descriptor.Base48())) {
+		// A metadata resource that is also read is not proven to be a full overwrite. Execute it
+		// conservatively instead of replacing the dispatch with a coarse full-surface clear.
+		if (cache.IsMeta(descriptor.Base48()) && (!resource.written || resource.read)) {
 			return false;
 		}
 	}

src/libs/agc.cpp

@@ -1220,7 +1220,7 @@ int KYTY_SYSV_ABI AgcGetGsOversubscription(ShaderRegister* regs, const Shader* g
 	constexpr uint32_t FULL_PC_OVERSUBSCRIPTION = 0x7ffu;
 	constexpr uint32_t FULL_SH_OVERSUBSCRIPTION = 0x007f0000u;
 
-	regs[0] = {Pm4::GE_PC_ALLOC, 0u};
+	regs[0] = {Pm4::UC_PARAMETER_OVERSUBSCRIPTION, 0u};
 	regs[1] = {Pm4::SPI_SHADER_PGM_RSRC4_GS, 0u};
 
 	if (budget == 0u) {

tests/ShaderRecompilerComputeTests.cpp

@@ -357,10 +357,23 @@ struct TextureCacheTestAccess {
     return cache.TryDownloadImage(id);
   }
 
+  static void RegisterHtileMeta(TextureCache &cache, uint64_t address) {
+    std::lock_guard lock(cache.m_lock);
+    auto &metadata = cache.m_surface_metas[address];
+    metadata.type = TextureCache::MetaDataInfo::Type::HTile;
+    metadata.clear_mask = 0;
+  }
+
   static TileManager &Tiler(TextureCache &cache) { return cache.m_tiler; }
 };
 
 struct RenderExecutorTestAccess {
+  static bool TryConsumeComputeMetaClear(RenderExecutor &executor,
+                                         const ShaderComputeInputInfo &input,
+                                         const CommandBuffer &buffer) {
+    return executor.TryConsumeComputeMetaClear(input, buffer);
+  }
+
   static TextureBinding
   ResolveTexture(RenderExecutor &executor,
                  const ShaderRecompiler::IR::ImageResource &resource,
@@ -3778,6 +3791,92 @@ public:
     std::printf("[host]    %-32s ok\n", name);
   }
 
+  void CheckComputeMetaClearClassification() {
+    constexpr const char *name = "ComputeMetaClearClassification";
+    constexpr uint64_t read_only_meta = 0x0000000204201f00ull;
+    constexpr uint64_t read_write_meta = 0x0000000204202000ull;
+    constexpr uint64_t write_only_meta = 0x0000000204202100ull;
+    EnsureRuntimeContext();
+    RenderContext context(m_runtime_context);
+    auto &scheduler = context.GetCommandScheduler();
+    HW::Context registers{};
+    HW::UserConfig user_config{};
+    HW::Shader shaders{};
+    scheduler.Begin(registers, user_config, shaders);
+    auto &texture_cache = context.GetTextureCache();
+    TextureCacheTestAccess::RegisterHtileMeta(texture_cache, read_only_meta);
+    TextureCacheTestAccess::RegisterHtileMeta(texture_cache, read_write_meta);
+    TextureCacheTestAccess::RegisterHtileMeta(texture_cache, write_only_meta);
+    Require(name, "HTile fixture",
+            texture_cache.IsMeta(read_only_meta) &&
+                texture_cache.IsMeta(read_write_meta) &&
+                texture_cache.IsMeta(write_only_meta) &&
+                !texture_cache.IsMetaCleared(read_only_meta, 0) &&
+                !texture_cache.IsMetaCleared(read_write_meta, 0) &&
+                !texture_cache.IsMetaCleared(write_only_meta, 0),
+            "test HTile entries were not registered in their initial state");
+
+    const auto MakeInput = [](uint64_t address, bool read, bool written) {
+      auto program = std::make_shared<ShaderRecompiler::IR::Program>();
+      program->stage = ShaderType::Compute;
+      ShaderRecompiler::IR::BufferResource resource{};
+      resource.read = read;
+      resource.written = written;
+      resource.formatted = true;
+      program->info.buffers.push_back(resource);
+
+      ShaderBufferResource descriptor{};
+      descriptor.UpdateAddress48(address);
+      descriptor.fields[1] |= 16u << 16u;
+      descriptor.fields[2] = 8;
+      descriptor.fields[3] =
+          DstSel(4, 5, 6, 7) |
+          (static_cast<uint32_t>(Prospero::BufferFormat::k32_32_32_32UInt)
+           << 12u);
+      ShaderRecompiler::IR::DescriptorValue value{};
+      value.dword_count = 4;
+      std::copy_n(descriptor.fields, value.dword_count, value.dwords.begin());
+      auto snapshot =
+          std::make_shared<ShaderRecompiler::IR::ResourceSnapshot>();
+      snapshot->buffers.push_back(value);
+
+      ShaderComputeInputInfo input{};
+      input.stage.program = std::move(program);
+      input.stage.resources = std::move(snapshot);
+      return input;
+    };
+    auto &executor = context.GetRenderExecutor();
+    auto &command = scheduler.Current();
+    const auto read_only_input = MakeInput(read_only_meta, true, false);
+    const bool read_only_consumed =
+        RenderExecutorTestAccess::TryConsumeComputeMetaClear(
+            executor, read_only_input, command);
+    Require(name, "read-only dispatch preserved",
+            !read_only_consumed &&
+                !texture_cache.IsMetaCleared(read_only_meta, 0),
+            "a metadata read-only dispatch changed logical clear state");
+
+    const auto read_write_input = MakeInput(read_write_meta, true, true);
+    const bool read_write_consumed =
+        RenderExecutorTestAccess::TryConsumeComputeMetaClear(
+            executor, read_write_input, command);
+    Require(name, "read/write dispatch preserved",
+            !read_write_consumed &&
+                !texture_cache.IsMetaCleared(read_write_meta, 0),
+            "a metadata read/modify/write dispatch was replaced by a clear");
+
+    const auto write_only_input = MakeInput(write_only_meta, false, true);
+    const bool write_only_consumed =
+        RenderExecutorTestAccess::TryConsumeComputeMetaClear(
+            executor, write_only_input, command);
+    Require(name, "write-only fill consumed",
+            write_only_consumed &&
+                texture_cache.IsMetaCleared(write_only_meta, 0),
+            "a metadata write-only fill was not consumed as a clear");
+    scheduler.Finish();
+    std::printf("[host]    %-32s ok\n", name);
+  }
+
   void CheckUnifiedTextureCacheFlow() {
     constexpr const char *name = "UnifiedTextureCacheFlow";
     constexpr uintptr_t base = 0x0000000200600000ull;
@@ -24556,10 +24655,10 @@ void CheckPm4NativeTargetGeometryRegisters(RenderContext &renderer) {
 
   // Removed GCN shader resource/checksum/queue registers. Numeric offsets keep
   // this check independent of the deleted legacy names.
-  constexpr std::array<uint32_t, 22> legacy_shader_slots{
-      0x000u, 0x001u, 0x002u, 0x003u, 0x030u, 0x081u, 0x0b0u, 0x0bcu,
-      0x100u, 0x101u, 0x130u, 0x14au, 0x14bu, 0x20eu, 0x20fu, 0x210u,
-      0x211u, 0x216u, 0x217u, 0x219u, 0x21au, 0x27du,
+  constexpr std::array<uint32_t, 19> legacy_shader_slots{
+      0x000u, 0x001u, 0x002u, 0x003u, 0x030u, 0x0b0u, 0x0bcu, 0x130u,
+      0x14au, 0x14bu, 0x20eu, 0x20fu, 0x210u, 0x211u, 0x216u, 0x217u,
+      0x219u, 0x21au, 0x27du,
   };
   for (const auto offset : legacy_shader_slots) {
     legacy_slots_are_unhandled &= g_hw_sh_func[offset] == nullptr &&
@@ -24570,12 +24669,6 @@ void CheckPm4NativeTargetGeometryRegisters(RenderContext &renderer) {
                                   g_hw_sh_indirect_func[offset] == nullptr;
   }
 
-  constexpr std::array<uint32_t, 1> legacy_uconfig_slots{0x0260u};
-  for (const auto offset : legacy_uconfig_slots) {
-    legacy_slots_are_unhandled &= g_hw_uc_func[offset] == nullptr &&
-                                  g_hw_uc_indirect_func[offset] == nullptr;
-  }
-
   std::array<uint32_t, 3> index_size_packet{
       KYTY_PM4(3, Pm4::IT_SET_UCONFIG_REG_INDEX, Pm4::R_ZERO),
       0x20000000u | Pm4::VGT_INDEX_TYPE,
@@ -24601,6 +24694,54 @@ void CheckPm4NativeTargetGeometryRegisters(RenderContext &renderer) {
   std::printf("[host]    %-32s ok\n", "Pm4NativeTargetGeometry");
 }
 
+void CheckPm4PrivateAgcShaderRegisters(RenderContext &renderer) {
+  GraphicsInitJmpTables();
+  CommandProcessor processor(renderer, 0);
+  std::array<uint32_t, 6> registers{
+      Pm4::SPI_SHADER_PGM_RSRC4_GS, 0x0badc0deu,
+      Pm4::SPI_SHADER_PGM_CHKSUM_HS, 0x12345678u,
+      Pm4::SPI_SHADER_PGM_RSRC4_HS, 0x87654321u,
+  };
+  const auto address = reinterpret_cast<uint64_t>(registers.data());
+  std::array<uint32_t, 5> command{
+      KYTY_PM4(5, Pm4::IT_SET_SH_REG_INDIRECT, Pm4::R_ZERO),
+      static_cast<uint32_t>(address), static_cast<uint32_t>(address >> 32u),
+      0x80000000u, 3u,
+  };
+  Pm4Execution execution;
+  const bool handlers_present =
+      g_hw_sh_indirect_func[Pm4::SPI_SHADER_PGM_RSRC4_GS] != nullptr &&
+      g_hw_sh_indirect_func[Pm4::SPI_SHADER_PGM_CHKSUM_HS] != nullptr &&
+      g_hw_sh_indirect_func[Pm4::SPI_SHADER_PGM_RSRC4_HS] != nullptr;
+  Require("Pm4PrivateAgcShaderRegisters", "private shader register stream",
+          handlers_present &&
+              processor.Process(execution, command) == Pm4ProcessResult::Complete,
+          "private AGC shader registers were rejected by SET_SH_REG_INDIRECT");
+  std::printf("[host]    %-32s ok\n", "Pm4PrivateAgcShaderRegisters");
+}
+
+void CheckPm4PrivateAgcUconfigRegisters(RenderContext &renderer) {
+  GraphicsInitJmpTables();
+  CommandProcessor processor(renderer, 0);
+  std::array<uint32_t, 2> registers{
+      Pm4::UC_PARAMETER_OVERSUBSCRIPTION, 0x12345678u,
+  };
+  const auto address = reinterpret_cast<uint64_t>(registers.data());
+  std::array<uint32_t, 5> command{
+      KYTY_PM4(5, Pm4::IT_SET_UCONFIG_REG_INDIRECT, Pm4::R_ZERO),
+      static_cast<uint32_t>(address), static_cast<uint32_t>(address >> 32u),
+      0x80000000u, 1u,
+  };
+  Pm4Execution execution;
+  const bool handler_present =
+      g_hw_uc_indirect_func[Pm4::UC_PARAMETER_OVERSUBSCRIPTION] != nullptr;
+  Require("Pm4PrivateAgcUconfigRegisters", "private UCONFIG register stream",
+          handler_present &&
+              processor.Process(execution, command) == Pm4ProcessResult::Complete,
+          "private AGC UCONFIG register was rejected by SET_UCONFIG_REG_INDIRECT");
+  std::printf("[host]    %-32s ok\n", "Pm4PrivateAgcUconfigRegisters");
+}
+
 void CheckPm4DirectShaderRegisterFallback(RenderContext &renderer) {
   GraphicsInitJmpTables();
   CommandProcessor processor(renderer, 0);
@@ -25361,6 +25502,8 @@ int main(int argc, char **argv) {
   if (argc == 2 && std::strcmp(argv[1], "--context-state-only") == 0) {
     VulkanHarness vulkan;
     CheckPm4NativeTargetGeometryRegisters(vulkan.RuntimeRenderer());
+    CheckPm4PrivateAgcShaderRegisters(vulkan.RuntimeRenderer());
+    CheckPm4PrivateAgcUconfigRegisters(vulkan.RuntimeRenderer());
     CheckPm4BlendColorRegisterRanges(vulkan.RuntimeRenderer());
     CheckPm4PolygonOffsetRegisters(vulkan.RuntimeRenderer());
     CheckPm4ContextStateOperations(vulkan.RuntimeRenderer());
@@ -25399,6 +25542,11 @@ int main(int argc, char **argv) {
     vulkan.CheckUnifiedTextureCacheFlow();
     return 0;
   }
+  if (argc == 2 && std::strcmp(argv[1], "--compute-meta-clear-only") == 0) {
+    VulkanHarness vulkan;
+    vulkan.CheckComputeMetaClearClassification();
+    return 0;
+  }
   if (argc == 2 && std::strcmp(argv[1], "--layered-image-only") == 0) {
     VulkanHarness vulkan;
     vulkan.CheckUnifiedImageViewCache();
@@ -25541,6 +25689,8 @@ int main(int argc, char **argv) {
   CheckPm4SyntheticOcclusionCounterDump(vulkan.RuntimeRenderer());
   CheckPm4StencilInfoValueLane(vulkan.RuntimeRenderer());
   CheckPm4NativeTargetGeometryRegisters(vulkan.RuntimeRenderer());
+  CheckPm4PrivateAgcShaderRegisters(vulkan.RuntimeRenderer());
+  CheckPm4PrivateAgcUconfigRegisters(vulkan.RuntimeRenderer());
   CheckPm4DirectShaderRegisterFallback(vulkan.RuntimeRenderer());
   CheckPm4GuardBandRegisterRanges(vulkan.RuntimeRenderer());
   CheckPm4BlendColorRegisterRanges(vulkan.RuntimeRenderer());