diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index 8742d462f..49757c4fb 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -1911,7 +1911,7 @@ private: } if ((positive ? op == ValueOpcode::ULessThan32 : op == ValueOpcode::UGreaterThanEqual32) && EquivalentValue(m_program, test->Arg(0), value)) return test->Arg(1); - if (!positive && op == ValueOpcode::ULessThanEqual32 && + if ((positive ? op == ValueOpcode::UGreaterThan32 : op == ValueOpcode::ULessThanEqual32) && EquivalentValue(m_program, test->Arg(1), value)) return test->Arg(0); const auto* scan = value.Resolve().TryInstruction(); if (finite_bitscan && scan != nullptr && scan->GetOpcode() == ValueOpcode::FindILsb32 && diff --git a/tests/ResourceTrackingTests.cpp b/tests/ResourceTrackingTests.cpp index 91ace92c7..63dbb9508 100644 --- a/tests/ResourceTrackingTests.cpp +++ b/tests/ResourceTrackingTests.cpp @@ -5385,7 +5385,10 @@ void TestGuardedWaterfallBufferRecords() { namespace CFG = Libs::Graphics::ShaderRecompiler::CFG; namespace Prospero = Libs::Graphics::Prospero; using Libs::Graphics::DstSel; - enum class Guard { Exact, Reversed, NoRange, NoEquality, OtherLocal, UnguardedUse, SeparateVotes }; + enum class Guard { + Exact, Reversed, Greater, GreaterEqual, WrongGreater, SignedGreater, + NoRange, NoEquality, OtherLocal, UnguardedUse, SeparateVotes + }; const auto make = [](Guard guard) { auto f = std::make_unique(); const auto count = f->UserData(2u); @@ -5399,11 +5402,22 @@ void TestGuardedWaterfallBufferRecords() { const auto initial = f->Emit(ValueOpcode::ULessThan32, {lane, Value(32u)}); const auto range_local = guard == Guard::OtherLocal ? f->Emit(ValueOpcode::IAdd32, {local, Value(1u)}) : local; - // Match the captured ANDN1_SAVEEXEC predicate, including its initial-EXEC witness. - const auto excluded = f->Emit(ValueOpcode::LogicalAnd, - {initial, f->Emit(ValueOpcode::ULessThanEqual32, {count, range_local})}); - const auto range = f->Emit(ValueOpcode::LogicalAnd, - {initial, f->Emit(ValueOpcode::LogicalNot, {excluded})}); + Value range; + if (guard == Guard::Greater || guard == Guard::GreaterEqual || + guard == Guard::WrongGreater || guard == Guard::SignedGreater) { + const auto op = guard == Guard::GreaterEqual ? ValueOpcode::UGreaterThanEqual32 + : guard == Guard::SignedGreater ? ValueOpcode::SGreaterThan32 : ValueOpcode::UGreaterThan32; + const auto compare = f->Emit(op, guard == Guard::WrongGreater + ? std::initializer_list{range_local, count} + : std::initializer_list{count, range_local}); + range = f->Emit(ValueOpcode::LogicalAnd, {initial, compare}); + } else { + // Match ANDN1_SAVEEXEC, including its initial-EXEC witness. + const auto excluded = f->Emit(ValueOpcode::LogicalAnd, + {initial, f->Emit(ValueOpcode::ULessThanEqual32, {count, range_local})}); + range = f->Emit(ValueOpcode::LogicalAnd, + {initial, f->Emit(ValueOpcode::LogicalNot, {excluded})}); + } const auto equal = f->Emit(ValueOpcode::IEqual32, guard == Guard::Reversed ? std::initializer_list{local, key} : std::initializer_list{key, local}); @@ -5450,7 +5464,8 @@ void TestGuardedWaterfallBufferRecords() { .userdata = &memory, .read_specialization_memory = ReadLinearTestMemory}; ResourceSnapshot snapshot; ResourceSpecialization specialization; - for (const auto guard : {Guard::Exact, Guard::Reversed, Guard::NoRange, Guard::NoEquality, + for (const auto guard : {Guard::Exact, Guard::Reversed, Guard::Greater, Guard::GreaterEqual, + Guard::WrongGreater, Guard::SignedGreater, Guard::NoRange, Guard::NoEquality, Guard::OtherLocal, Guard::UnguardedUse, Guard::SeparateVotes}) { auto fixture = make(guard); const auto plan = ExtractResourcePlan(fixture->program); @@ -5458,14 +5473,14 @@ void TestGuardedWaterfallBufferRecords() { return plan.descriptor_sources[buffer.source].indirect_descriptor.has_value(); }); Check(root != plan.info.buffers.end(), "waterfall buffer lost its indirect resource"); - const bool proven = guard == Guard::Exact || guard == Guard::Reversed; + const bool proven = guard == Guard::Exact || guard == Guard::Reversed || guard == Guard::Greater; const auto& domain = *plan.descriptor_sources[root->source].indirect_descriptor; Check(domain.key_count.IsEmpty() != proven && !domain.selector && domain.table_stride == 488u && domain.table_immediate == 216u, "waterfall bound mixed different lanes, values or resource uses"); const auto materialize = [&] { return MaterializeResources(plan, runtime, snapshot, specialization); }; if (!proven) { - Check(!materialize(), "unproved waterfall skipped a reachable unsupported descriptor"); + Check(!materialize(), "unproved waterfall bypassed conservative descriptor rejection"); continue; } for (const uint32_t count : {2u, 0u, 1u, 2u}) { diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 73d1e16e5..d53f0a2d7 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -37987,11 +37987,15 @@ void CheckGuardedWaterfallBufferRecords(VulkanHarness &vulkan) { .limits.minStorageBufferOffsetAlignment; const u32 slot = (4096u + alignment - 1u) / alignment * alignment; constexpr u32 selectors = 256u, table = 512u; + for (const bool direct_gt : {false, true}) for (const auto [wave, upper_only] : {std::pair{32u, false}, std::pair{64u, false}, std::pair{64u, true}}) { TestCase test; - test.name = upper_only ? "GuardedWaterfallRecordsUpper64" : wave == 32u - ? "GuardedWaterfallRecords32" : "GuardedWaterfallRecords64"; + test.name = direct_gt + ? upper_only ? "GuardedWaterfallGtRecordsUpper64" : wave == 32u + ? "GuardedWaterfallGtRecords32" : "GuardedWaterfallGtRecords64" + : upper_only ? "GuardedWaterfallRecordsUpper64" : wave == 32u + ? "GuardedWaterfallRecords32" : "GuardedWaterfallRecords64"; test.initial.assign((slot * 3u + wave * 16u + 16u) / 4u, 0xdeadbeefu); std::fill_n(test.initial.begin() + table / 4u, 976u / 4u, 0u); constexpr std::array poison{0x012b6890u, 0x501502f9u, 0x493ba30eu, 0x3ea2f352u}; @@ -38024,8 +38028,10 @@ void CheckGuardedWaterfallBufferRecords(VulkanHarness &vulkan) { AppendSMovLiteral(&code, 127, UINT32_MAX); } code.insert(code.end(), {EncodeSop1(0x04, 20, 126), - EncodeMubuf0(0x0c, 0, true, false), EncodeMubuf1(5, 1, 30), EncodeSopp(0x0c, 0), - EncodeVopc(0xc3, 2, 5), EncodeSop1(0x37, 52, 106)}); // Exclude N <= local key. + EncodeMubuf0(0x0c, 0, true, false), EncodeMubuf1(5, 1, 30), EncodeSopp(0x0c, 0)}); + if (direct_gt) code.push_back(EncodeVopc(0xd4, 2, 5)); // Keep N > local key in EXEC. + else code.insert(code.end(), {EncodeVopc(0xc3, 2, 5), + EncodeSop1(0x37, 52, 106)}); // Exclude N <= local key. const auto empty = code.size(); code.push_back(0u); code.push_back(EncodeSop1(0x04, 54, 126)); @@ -38047,6 +38053,7 @@ void CheckGuardedWaterfallBufferRecords(VulkanHarness &vulkan) { AppendVMovLiteral(&code, 9, 0x76543210u); code.insert(code.end(), {EncodeMubuf0(0x1c, 12u, true, false), EncodeMubuf1(9, 4, 30)}); AppendEnd(&code); + test.decoded_counts = {{direct_gt ? "V_CMPX_GT_U32" : "V_CMP_LE_U32", 1u}}; test.required_spirv = {"OpLoopMerge", "OpGroupNonUniformShuffle", "Volatile"}; test.forbidden_spirv = {"load_indirect_buffer_", "indirect_buffer_format_geometry"}; const auto compiled = CompileCase(test, vulkan.SubgroupSize()); @@ -38064,9 +38071,10 @@ void CheckGuardedWaterfallBufferRecords(VulkanHarness &vulkan) { producer.compute_info.threads_num[2] = 1u; producer.compute_info.lds_size_dwords = 0; test.expected = test.initial; + // Exclude key == N and the key that wraps to the unsupported descriptor at byte 480. + constexpr std::array keys{0u, 1u, 2u, 0x029f79b5u}; for (u32 lane = 0; lane < wave; ++lane) { - // This excluded key would wrap to the unsupported descriptor at byte480. - const u32 key = lane % 3u == 2u ? 0x029f79b5u : lane % 3u; + const u32 key = keys[lane % keys.size()]; AppendVMovLiteral(&producer.code, 1, key); AppendVMovLiteral(&producer.code, 2, selectors + lane * 4u); AppendBufferStoreDword(&producer.code, 1, 2); @@ -38083,7 +38091,7 @@ void CheckGuardedWaterfallBufferRecords(VulkanHarness &vulkan) { auto buffer = vulkan.CreateStorageBuffer(test.name, test.initial, test.initial.size()); vulkan.Dispatch(producer, producer_shader, buffer); vulkan.Dispatch(test, compiled, buffer); - CompareWords(test, "same-lane bound, GPU keys, wrapped poison, EXEC restoration and canaries", + CompareWords(test, "same-lane bound, key == N, wrapped poison, EXEC restoration and canaries", test.expected, vulkan.ReadBuffer(test.name, buffer, test.expected.size())); vulkan.DestroyBuffer(&buffer); }