diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index aab04bd71..7dfe28766 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -620,36 +620,6 @@ bool BuildSamplerPlan(const ShaderInfo& base, const Images& images, SamplerPlan& return true; } -static bool UniformIntegerValue(const ResourcePlan& program, Value root) { - if (!ValidateRuntimeValue(program, root)) { - return false; - } - std::vector pending {root}; - std::unordered_set visited; - while (!pending.empty()) { - const auto value = pending.back().Resolve(); - pending.pop_back(); - // Host floating-point evaluation does not model the shader's rounding/denormal modes. - if (TypesOverlap(value.GetType(), Type::F16 | Type::F32 | Type::F32x2)) { - return false; - } - const auto* inst = value.TryInstruction(); - if (inst != nullptr && visited.insert(inst).second) { - if (inst->GetOpcode() == ValueOpcode::ReadConst) { - const auto& read = program.srt_reads[inst->Arg(1).Resolve().U32()]; - if (!ValidateRuntimeValue(program, read.value)) { - return false; - } - pending.push_back(read.value); - } - for (size_t i = 0; i < inst->NumArgs(); i++) { - pending.push_back(inst->Arg(i)); - } - } - } - return true; -} - static std::vector ResourceControlFlow(const Program& program) { if (program.blocks.size() != program.block_info.size()) { return {}; @@ -670,7 +640,7 @@ static std::vector ResourceControlFlow(const Program& program) { case CFG::TerminatorKind::Branch: successors.push_back(terminator.true_block); break; case CFG::TerminatorKind::ConditionalBranch: successors = {terminator.true_block, terminator.false_block}; - if (UniformIntegerValue(program, info.condition)) { + if (ValidateRuntimeValue(program, info.condition, RuntimeValueType::Integer)) { block.condition = info.condition; } break; @@ -820,8 +790,12 @@ static UniformFillPlan AnalyzeUniformFill(const Program& program) { if (address == nullptr || address->GetOpcode() != ValueOpcode::MakeImageAddress) return {}; for (uint32_t axis = 0; axis < 3; ++axis) { const auto index = FillIndex(address->Arg(axis), axis); - if (!index || (*index)[0] != 0 || (*index)[1] != (axis < 2 ? 1u : 0u) || - (*index)[2] == 0 || (axis == 2 && (*index)[2] != 1)) return {}; + if (!index || (*index)[0] != 0) return {}; + if (axis < 2) { + if ((*index)[1] != 1 || (*index)[2] == 0) return {}; + } else if ((*index)[1] != 0 || (*index)[2] != 1) { + return {}; + } result.fill.group_stride[axis] = static_cast((*index)[2]); } const auto* values = store->Arg(2).ResolveInstruction(); @@ -855,12 +829,14 @@ static UniformFillPlan AnalyzeUniformFill(const Program& program) { constexpr std::array composites {ValueOpcode::CompositeConstructU32x2, ValueOpcode::CompositeConstructU32x3, ValueOpcode::CompositeConstructU32x4}; + if (result.fill.words > 1 && + (vector == nullptr || vector->GetOpcode() != composites[result.fill.words - 2])) + return {}; for (uint32_t i = 0; i < result.fill.words; ++i) { - if (result.fill.words > 1 && - (vector == nullptr || vector->GetOpcode() != composites[result.fill.words - 2])) - return {}; const auto word = result.fill.words == 1 ? data : vector->Arg(i); - if (word.GetType() != Type::U32 || !UniformIntegerValue(program, word)) return {}; + if (word.GetType() != Type::U32 || + !ValidateRuntimeValue(program, word, RuntimeValueType::Integer)) + return {}; result.values[i] = word; } return result; diff --git a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp index 3f64a2c32..5e31150dd 100644 --- a/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp +++ b/src/graphics/shader/recompiler/ir/passes/SrtWalker.cpp @@ -134,16 +134,29 @@ bool IsRuntimeUniformOp(ValueOpcode op) { class RuntimeValidator { public: - explicit RuntimeValidator(const ResourcePlan& program, Value active_mask = {}) - : m_program(program), m_active_mask(active_mask.Resolve()) {} + explicit RuntimeValidator(const ResourcePlan& program, RuntimeValueType type) + : m_program(program), m_type(type) {} bool Run(Value value) { return Validate(value); } private: - bool Validate(Value value) { - value = value.Resolve(); + bool ValidateArguments(const Inst& inst, bool require_uniform) { + for (size_t index = 0; index < inst.NumArgs(); index++) { + if (!Validate(inst.Arg(index), require_uniform)) return false; + } + return true; + } + + bool Validate(Value value, bool require_uniform = true) { + value = value.Resolve(); + // Host floating-point evaluation does not model shader rounding/denormal modes. + if (m_type == RuntimeValueType::Integer && + TypesOverlap(value.GetType(), Type::F16 | Type::F32 | Type::F32x2)) { + return false; + } const auto* inst = value.TryInstruction(); if (inst == nullptr) { + if (!require_uniform) return true; switch (value.GetType()) { case Type::U1: case Type::U8: @@ -155,15 +168,37 @@ private: } } if (!m_visiting.insert(inst).second) { - return false; + return !require_uniform; } const auto finish = [&](bool valid) { m_visiting.erase(inst); return valid; }; const auto op = inst->GetOpcode(); + if (op == ValueOpcode::ReadConst) { + const auto slot = inst->NumArgs() == 2 ? inst->Arg(1).Resolve() : Value {}; + if (inst->NumArgs() != 2 || inst->Arg(0).Resolve().TryInstruction() == nullptr || + inst->Arg(0).Resolve().TryInstruction()->GetOpcode() != + ValueOpcode::GetSrtResource || + !slot.IsImmediate() || slot.GetType() != Type::U32 || + slot.U32() >= m_program.srt_reads.size()) { + return finish(false); + } + if (m_type == RuntimeValueType::Integer) { + const auto active_mask = m_active_mask; + m_active_mask = {}; + const bool valid = Validate(m_program.srt_reads[slot.U32()].value); + m_active_mask = active_mask; + if (!valid) return finish(false); + } + } + if (!require_uniform) return finish(ValidateArguments(*inst, false)); if (!m_active_mask.IsEmpty() && IsRuntimeSelect(op) && inst->NumArgs() == 3 && inst->Arg(0).Resolve() == m_active_mask) { + // Empty EXEC reads lane zero, so ignored operands still require integer types. + if (m_type == RuntimeValueType::Integer && !Validate(inst->Arg(2), false)) { + return finish(false); + } return finish(Validate(inst->Arg(1))); } if (op == ValueOpcode::UndefU1 || op == ValueOpcode::UndefU8 || @@ -189,6 +224,9 @@ private: return finish(true); } if (op == ValueOpcode::Phi) { + if (m_type == RuntimeValueType::Integer && !ValidateArguments(*inst, false)) { + return finish(false); + } const auto invariant = ResolveInvariantPhi(m_program, value); if (invariant.IsEmpty()) { return finish(false); @@ -200,7 +238,14 @@ private: inst->Arg(1).GetType() != Type::U1) { return finish(false); } - return finish(RuntimeValidator(m_program, inst->Arg(1)).Run(inst->Arg(0))); + if (m_type == RuntimeValueType::Integer && !Validate(inst->Arg(1), false)) { + return finish(false); + } + const auto active_mask = m_active_mask; + m_active_mask = inst->Arg(1).Resolve(); + const bool valid = Validate(inst->Arg(0)); + m_active_mask = active_mask; + return finish(valid); } if (op == ValueOpcode::GetSrtResource) { if (inst->NumArgs() != 0) { @@ -208,16 +253,7 @@ private: } return finish(true); } - if (op == ValueOpcode::ReadConst) { - const auto slot = inst->NumArgs() == 2 ? inst->Arg(1).Resolve() : Value {}; - if (inst->NumArgs() != 2 || inst->Arg(0).Resolve().TryInstruction() == nullptr || - inst->Arg(0).Resolve().TryInstruction()->GetOpcode() != - ValueOpcode::GetSrtResource || - !slot.IsImmediate() || slot.GetType() != Type::U32 || - slot.U32() >= m_program.srt_reads.size()) { - return finish(false); - } - } else if (op == ValueOpcode::LoadAddressU32 || op == ValueOpcode::ReadConstBuffer) { + if (op == ValueOpcode::LoadAddressU32 || op == ValueOpcode::ReadConstBuffer) { const auto expected = op == ValueOpcode::LoadAddressU32 ? ValueOpcode::GetAddressResource : ValueOpcode::GetBufferResource; @@ -243,10 +279,12 @@ private: } if (op == ValueOpcode::GetBufferResource || op == ValueOpcode::GetImageResource || op == ValueOpcode::GetSamplerResource || op == ValueOpcode::GetAddressResource) { - const size_t expected = op == ValueOpcode::GetBufferResource ? 4u - : op == ValueOpcode::GetImageResource ? 8u - : op == ValueOpcode::GetSamplerResource ? 4u - : 2u; + size_t expected = 4u; + if (op == ValueOpcode::GetImageResource) { + expected = 8u; + } else if (op == ValueOpcode::GetAddressResource) { + expected = 2u; + } if (inst->NumArgs() != expected) { return finish(false); } @@ -254,15 +292,11 @@ private: op != ValueOpcode::LoadAddressU32 && !IsRuntimeUniformOp(op)) { return finish(false); } - for (size_t index = 0; index < inst->NumArgs(); index++) { - if (!Validate(inst->Arg(index))) { - return finish(false); - } - } - return finish(true); + return finish(ValidateArguments(*inst, true)); } const ResourcePlan& m_program; + RuntimeValueType m_type; Value m_active_mask; std::unordered_set m_visiting; }; @@ -952,7 +986,10 @@ bool EvaluateRuntimeSourcesImpl(const ResourcePlan& program, std::span active(program.descriptor_sources.size(), 1u); + std::vector active; + if (evaluate_flat) { + active.assign(program.descriptor_sources.size(), 1u); + } if (evaluate_flat && !program.control_flow.empty()) { for (const auto& block: program.control_flow) { for (const auto source: block.sources) { @@ -991,9 +1028,11 @@ bool EvaluateRuntimeSourcesImpl(const ResourcePlan& program, std::spandword_count; - for (uint32_t index = 0; active[source_index] && index < source->dword_count; index++) { - if (!evaluator.Evaluate(source->dwords[index], value.dwords[index])) { - return false; + if (!evaluate_flat || active[source_index]) { + for (uint32_t index = 0; index < source->dword_count; index++) { + if (!evaluator.Evaluate(source->dwords[index], value.dwords[index])) { + return false; + } } } evaluated.push_back(value); @@ -1021,8 +1060,8 @@ bool EvaluateRuntimeSourcesImpl(const ResourcePlan& program, std::span values, const SrtRuntime& runtime, std::span results); diff --git a/tests/ResourceMaterializationTests.cpp b/tests/ResourceMaterializationTests.cpp index fd2a0118a..d0567254e 100644 --- a/tests/ResourceMaterializationTests.cpp +++ b/tests/ResourceMaterializationTests.cpp @@ -159,6 +159,55 @@ void TestMappedSrtUsesDirectReaderByDefault() { "cache rematerialization did not use the direct reader by default"); } +void TestIntegerRuntimeValueFollowsSrtReads() { + using namespace Libs::Graphics::ShaderRecompiler::IR; + auto plan = SrtPlan(0x10000); + const auto root = plan.descriptor_sources.front().dwords[0]; + Check(ValidateRuntimeValue(plan, root, RuntimeValueType::Integer), + "integer SRT read was rejected"); + + Block values; + auto &comparison = values.AppendNewInst(ValueOpcode::FPOrdLessThanEqual32, + {Value::F32(1.f), Value::F32(0.f)}); + auto &selection = values.AppendNewInst( + ValueOpcode::SelectU32, {Value(&comparison), Value(1u), Value(0u)}); + plan.srt_reads[0].value = Value(&selection); + Check(ValidateRuntimeValue(plan, root), + "ordinary SRT validation rejected a floating-point dependency"); + Check(!ValidateRuntimeValue(plan, root, RuntimeValueType::Integer), + "integer SRT validation missed a hidden floating-point dependency"); + + auto &first = + values.AppendNewInst(ValueOpcode::ReadFirstLane, {root, Value(true)}); + Check(!ValidateRuntimeValue(plan, Value(&first), RuntimeValueType::Integer), + "read-first-lane lost integer-only SRT validation"); + + auto &active = values.AppendNewInst(ValueOpcode::ReadFirstLane, + {Value(&selection), Value(&comparison)}); + Check(!ValidateRuntimeValue(plan, Value(&active), RuntimeValueType::Integer), + "floating-point execution mask was accepted as integer-only"); + + auto &lane = values.AppendNewInst( + ValueOpcode::GetBuiltin, + {Value(static_cast(StageInputKind::LocalInvocationId)), + Value(0u)}); + auto &mask = + values.AppendNewInst(ValueOpcode::INotEqual32, {Value(&lane), Value(0u)}); + selection.SetArg(0, Value(&mask)); + active.SetArg(1, Value(&mask)); + Check(ValidateRuntimeValue(plan, Value(&active), RuntimeValueType::Integer), + "nonuniform integer execution mask was rejected"); + auto &float_value = + values.AppendNewInst(ValueOpcode::BitCastU32F32, {Value::F32(1.f)}); + selection.SetArg(2, Value(&float_value)); + Check(!ValidateRuntimeValue(plan, Value(&active), RuntimeValueType::Integer), + "floating-point inactive arm was accepted as integer-only"); + + plan.srt_reads[0].value = Value(&first); + Check(!ValidateRuntimeValue(plan, root, RuntimeValueType::Integer), + "cyclic SRT read-first-lane dependency was accepted"); +} + void TestUnbasedFlatCacheHitMaterializes() { using namespace Libs::Graphics::ShaderRecompiler::IR; auto plan = UnbasedFlatPlan(); @@ -217,6 +266,7 @@ void DbgExit(int) { std::abort(); } int main() { TestMappedSrtUsesDirectReaderByDefault(); + TestIntegerRuntimeValueFollowsSrtReads(); TestUnbasedFlatCacheHitMaterializes(); TestFailedMaterializationPreservesPriorStage(); TestMixedSamplerDuplicatesTheCorrectSnapshot(); diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 94ae80213..ce0cb7a88 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -1681,9 +1681,10 @@ void TestSopkCompareImmediateExtension() { Decoder::Program decoded; Decoder::DecodeProgram(shader, decoded); const auto &operand = decoded.instructions.front().src1; - const int32_t expected = opcode < 0x09 - ? (immediate == 0x8000u ? -32768 : -1) - : static_cast(immediate); + int32_t expected = static_cast(immediate); + if (opcode < 0x09) { + expected = immediate == 0x8000u ? -32768 : -1; + } Check(operand.value == static_cast(expected) && operand.signed_val == expected, "SOPK compare immediate extension does not match its signedness"); @@ -6057,10 +6058,23 @@ void TestMaskedValueDemand() { {zero, zero, zero, zero, zero, zero, zero, zero}); auto &sampler = entry.AppendNewInst(ValueOpcode::GetSamplerResource, {zero, zero, zero, zero}); - program.memory_info.emplace_back().image_sample_flags = - use == Use::ExplicitLod ? ShaderRecompiler::Decoder::ImageSampleFlagLod : - use == Use::ExplicitGradient ? ShaderRecompiler::Decoder::ImageSampleFlagDerivative : - use == Use::LevelZero ? ShaderRecompiler::Decoder::ImageSampleFlagLevelZero : 0u; + auto &memory = program.memory_info.emplace_back(); + switch (use) { + case Use::ExplicitLod: + memory.image_sample_flags = + ShaderRecompiler::Decoder::ImageSampleFlagLod; + break; + case Use::ExplicitGradient: + memory.image_sample_flags = + ShaderRecompiler::Decoder::ImageSampleFlagDerivative; + break; + case Use::LevelZero: + memory.image_sample_flags = + ShaderRecompiler::Decoder::ImageSampleFlagLevelZero; + break; + default: + break; + } value = Value(&join.AppendNewInst(ValueOpcode::ImageSampleRaw, {Value(&image), Value(&sampler), Value(&address)})); value = Value(&join.AppendNewInst(ValueOpcode::CompositeExtractU32x4, {value, Value(0u)}));