diff --git a/src/graphics/shader/recompiler/backend/spirv/SpirvEmitter.cpp b/src/graphics/shader/recompiler/backend/spirv/SpirvEmitter.cpp index 7ca1947b..6266a2ad 100644 --- a/src/graphics/shader/recompiler/backend/spirv/SpirvEmitter.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/SpirvEmitter.cpp @@ -13,51 +13,6 @@ using ShaderError::Fail; namespace { -bool ImageBinding(const IR::ImageResource& image, IR::DescriptorBindingKind& kind) { - using Kind = IR::DescriptorBindingKind; - using Dim = Decoder::ImageDimension; - if (image.kind == IR::ResourceKind::Image || image.kind == IR::ResourceKind::ImageUint) { - const bool integer = image.kind == IR::ResourceKind::ImageUint; - switch (image.dimension) { - case Dim::Dim1D: kind = integer ? Kind::SampledUint1D : Kind::Sampled1D; return true; - case Dim::Dim1DArray: - kind = integer ? Kind::SampledUint1DArray : Kind::Sampled1DArray; - return true; - case Dim::Dim2D: kind = integer ? Kind::SampledUint2D : Kind::Sampled2D; return true; - case Dim::Dim2DMsaa: - kind = integer ? Kind::SampledUint2DMsaa : Kind::Sampled2DMsaa; - return true; - case Dim::Dim3D: kind = integer ? Kind::SampledUint3D : Kind::Sampled3D; return true; - case Dim::Dim2DArray: - kind = integer ? Kind::SampledUint2DArray : Kind::Sampled2DArray; - return true; - case Dim::Dim2DMsaaArray: - kind = integer ? Kind::SampledUint2DMsaaArray : Kind::Sampled2DMsaaArray; - return true; - case Dim::Unknown: return false; - } - } - const bool uint_image = image.kind == IR::ResourceKind::StorageImageUint; - if (image.kind != IR::ResourceKind::StorageImage && !uint_image) { - return false; - } - switch (image.dimension) { - case Dim::Dim1D: kind = uint_image ? Kind::StorageUint1D : Kind::Storage1D; return true; - case Dim::Dim1DArray: - kind = uint_image ? Kind::StorageUint1DArray : Kind::Storage1DArray; - return true; - case Dim::Dim2D: kind = uint_image ? Kind::StorageUint2D : Kind::Storage2D; return true; - case Dim::Dim3D: kind = uint_image ? Kind::StorageUint3D : Kind::Storage3D; return true; - case Dim::Dim2DArray: - kind = uint_image ? Kind::StorageUint2DArray : Kind::Storage2DArray; - return true; - case Dim::Dim2DMsaa: - case Dim::Dim2DMsaaArray: return false; - case Dim::Unknown: return false; - } - return false; -} - bool ValidateNativeProgram(const IR::Program& program, std::string* error) { using Kind = IR::DescriptorBindingKind; constexpr auto KindCount = static_cast(Kind::Count); @@ -79,18 +34,18 @@ bool ValidateNativeProgram(const IR::Program& program, std::string* error) { Expect(Kind::Buffers, Dense(program.info.buffers.size())); } for (uint32_t i = 0; i < program.info.images.size(); i++) { - Kind kind; - if (!ImageBinding(program.info.images[i], kind)) { + const auto kind = IR::DescriptorBindingForImage(program.info.images[i]); + if (!kind.has_value()) { return Fail(error, "native shader plan has an invalid image class"); } - present[static_cast(kind)] = true; + present[static_cast(*kind)] = true; const auto dynamic = program.info.images[i].mip_mode == IR::ImageMipMode::DynamicStorage; const auto count = dynamic ? program.info.images[i].mip_count : 1u; if (count == 0u || (!dynamic && program.info.images[i].mip_count != 1u)) { return Fail(error, "native shader plan has an invalid image mip descriptor count"); } - expected[static_cast(kind)].insert(expected[static_cast(kind)].end(), count, - i); + expected[static_cast(*kind)].insert(expected[static_cast(*kind)].end(), + count, i); } if (!program.info.samplers.empty()) { Expect(Kind::Samplers, Dense(program.info.samplers.size())); diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAlu.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAlu.cpp index 2ad9cdb1..bb32d3b5 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAlu.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterAlu.cpp @@ -520,13 +520,6 @@ bool EmitValueAlu(ValueEmitContext& ctx, const IR::Inst& inst) { case IR::ValueOpcode::FindUMsb64: ctx.Define(inst, EmitFindMsb64(state, ctx.Arg(inst, 0))); return true; - case IR::ValueOpcode::FindSMsb32: { - const auto value = - EmitExt(state, TypeI32(state), 74u, - {NewUnary(state, OpBitcast, TypeI32(state), ctx.Arg(inst, 0))}); - ctx.Define(inst, NewUnary(state, OpBitcast, TypeU32(state), value)); - return true; - } case IR::ValueOpcode::SMin32: ctx.Define(inst, EmitMinMaxI32Value(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), false)); return true; @@ -591,26 +584,6 @@ bool EmitValueAlu(ValueEmitContext& ctx, const IR::Inst& inst) { ctx.Define(inst, CompareOrdered64(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), OpUGreaterThan, OpUGreaterThan)); return true; - case IR::ValueOpcode::SGreaterThan64: - ctx.Define(inst, CompareOrdered64(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), - OpSGreaterThan, OpUGreaterThan)); - return true; - case IR::ValueOpcode::ULessThanEqual64: - ctx.Define(inst, CompareOrdered64(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), - OpULessThan, OpULessThanEqual)); - return true; - case IR::ValueOpcode::SLessThanEqual64: - ctx.Define(inst, CompareOrdered64(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), - OpSLessThan, OpULessThanEqual)); - return true; - case IR::ValueOpcode::UGreaterThanEqual64: - ctx.Define(inst, CompareOrdered64(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), - OpUGreaterThan, OpUGreaterThanEqual)); - return true; - case IR::ValueOpcode::SGreaterThanEqual64: - ctx.Define(inst, CompareOrdered64(state, ctx.Arg(inst, 0), ctx.Arg(inst, 1), - OpSGreaterThan, OpUGreaterThanEqual)); - return true; case IR::ValueOpcode::LogicalOr: return binary(OpLogicalOr, IR::Type::U1); case IR::ValueOpcode::LogicalAnd: return binary(OpLogicalAnd, IR::Type::U1); case IR::ValueOpcode::LogicalXor: return binary(OpLogicalNotEqual, IR::Type::U1); diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterFlow.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterFlow.cpp index 49ccdb79..aaf17227 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterFlow.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterFlow.cpp @@ -363,8 +363,7 @@ void EmitExport(ValueEmitContext& ctx, const IR::Inst& inst) { bool EmitValueFlow(ValueEmitContext& ctx, const IR::Inst& inst) { auto& state = ctx.state; switch (inst.GetOpcode()) { - case IR::ValueOpcode::Identity: - case IR::ValueOpcode::ConditionRef: ctx.Define(inst, ctx.Arg(inst, 0)); return true; + case IR::ValueOpcode::Identity: ctx.Define(inst, ctx.Arg(inst, 0)); return true; case IR::ValueOpcode::Void: case IR::ValueOpcode::Reference: case IR::ValueOpcode::ReferenceU32: diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp index 4ce3f14e..8c47618f 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterImage.cpp @@ -345,8 +345,9 @@ uint32_t ImageAtomicOpcode(IR::ValueOpcode opcode) { } // namespace bool EmitValueImage(ValueEmitContext& ctx, const IR::Inst& inst) { - const auto op = inst.GetOpcode(); - if (op < IR::ValueOpcode::ImageQueryDimensions || op > IR::ValueOpcode::ImageAtomicXor32) { + const auto op = inst.GetOpcode(); + const auto image_info = IR::ImageOpcodeInfoOf(op); + if (image_info.access == IR::ImageAccess::None) { return false; } auto& state = ctx.state; @@ -354,10 +355,7 @@ bool EmitValueImage(ValueEmitContext& ctx, const IR::Inst& inst) { const auto pc = inst.Flags().pc; const auto image_arg = inst.Arg(0); ctx.ResourceIndex(image_arg, IR::ValueOpcode::GetImageResource); - const bool sampled = op == IR::ValueOpcode::ImageQueryLod || - op == IR::ValueOpcode::ImageSampleRaw || - op == IR::ValueOpcode::ImageGatherRaw; - const auto* address = ctx.ImageAddress(inst.Arg(sampled ? 2 : 1)); + const auto* address = ctx.ImageAddress(inst.Arg(image_info.needs_sampler ? 2 : 1)); if (address == nullptr) return true; if (op == IR::ValueOpcode::ImageQueryDimensions) { state.builder.RequireCapability(CapabilityImageQuery); @@ -392,7 +390,8 @@ bool EmitValueImage(ValueEmitContext& ctx, const IR::Inst& inst) { ctx.Define( inst, EmitValueOrDefaultIfCondition( - state, condition, TypeU32Vector(state, 4), ConstantU32CompositeZero(state, 4), [&]() { + state, condition, TypeU32Vector(state, 4), ConstantU32CompositeZero(state, 4), + [&]() { const auto image = LoadSampledImageDescriptor(state, mem, pc, view); const auto color = state.builder.AllocateId(); const auto coord = CoordU32(ctx, mem, *address, view); diff --git a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp index bbfea504..b5e7b1ec 100644 --- a/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp +++ b/src/graphics/shader/recompiler/backend/spirv/spirvEmitterMemory.cpp @@ -867,10 +867,9 @@ uint32_t LoadWideShared(ValueEmitContext& ctx, const IR::Inst& inst, uint32_t co const auto base = ByteAddress(ctx, inst, mem); std::array values {}; for (uint32_t component = 0; component < components; component++) { - const auto address = component == 0u - ? base - : Binary(state, OpIAdd, TypeU32(state), base, - ConstantU32(state, component * 4u)); + const auto address = component == 0u ? base + : Binary(state, OpIAdd, TypeU32(state), base, + ConstantU32(state, component * 4u)); const auto raw_index = Binary(state, OpShiftRightLogical, TypeU32(state), address, ConstantU32(state, 2)); const auto access = PrepareMemoryElement(ctx, mem, resource, raw_index); @@ -889,10 +888,9 @@ void StoreWideShared(ValueEmitContext& ctx, const IR::Inst& inst, uint32_t compo const auto resource = PrepareMemoryResourceAccess(state, mem); const auto base = ByteAddress(ctx, inst, mem); for (uint32_t component = 0; component < components; component++) { - const auto address = component == 0u - ? base - : Binary(state, OpIAdd, TypeU32(state), base, - ConstantU32(state, component * 4u)); + const auto address = component == 0u ? base + : Binary(state, OpIAdd, TypeU32(state), base, + ConstantU32(state, component * 4u)); const auto raw_index = Binary(state, OpShiftRightLogical, TypeU32(state), address, ConstantU32(state, 2)); const auto access = PrepareMemoryElement(ctx, mem, resource, raw_index); @@ -963,13 +961,12 @@ bool EmitValueMemory(ValueEmitContext& ctx, const IR::Inst& inst) { })); return true; } - const bool load_address = op == IR::ValueOpcode::LoadAddressU8 || - op == IR::ValueOpcode::LoadAddressU16 || - op == IR::ValueOpcode::LoadAddressU32; + const auto address_info = IR::AddressOpcodeInfoOf(op); + const bool load_address = address_info.access == IR::AddressAccess::Read; const bool load_buffer = op == IR::ValueOpcode::LoadBufferU8 || op == IR::ValueOpcode::LoadBufferU16 || op == IR::ValueOpcode::LoadBufferU32; - const bool load_shared = IR::SharedAccessOf(op) == IR::SharedAccess::Read; + const bool load_shared = IR::SharedAccessOf(op) == IR::SharedAccess::Read; if (load_address || load_buffer || load_shared) { const auto mem = ctx.Memory(inst); uint32_t value = 0; @@ -986,13 +983,11 @@ bool EmitValueMemory(ValueEmitContext& ctx, const IR::Inst& inst) { ctx.Define(inst, value); return true; } - const bool store_address = op == IR::ValueOpcode::StoreAddressU8 || - op == IR::ValueOpcode::StoreAddressU16 || - op == IR::ValueOpcode::StoreAddressU32; + const bool store_address = address_info.access == IR::AddressAccess::Write; const bool store_buffer = op == IR::ValueOpcode::StoreBufferU8 || op == IR::ValueOpcode::StoreBufferU16 || op == IR::ValueOpcode::StoreBufferU32; - const bool store_shared = IR::SharedAccessOf(op) == IR::SharedAccess::Write; + const bool store_shared = IR::SharedAccessOf(op) == IR::SharedAccess::Write; if (store_address || store_buffer || store_shared) { const auto mem = ctx.Memory(inst); if (op == IR::ValueOpcode::StoreBufferU32 && mem.formatted) diff --git a/src/graphics/shader/recompiler/ir/ShaderIR.h b/src/graphics/shader/recompiler/ir/ShaderIR.h index 4c660ea6..eef456e9 100644 --- a/src/graphics/shader/recompiler/ir/ShaderIR.h +++ b/src/graphics/shader/recompiler/ir/ShaderIR.h @@ -7,6 +7,7 @@ #include "graphics/shader/recompiler/frontend/cfg/ShaderCFG.h" #include "graphics/shader/recompiler/frontend/decode/ShaderDecoder.h" #include "graphics/shader/recompiler/ir/opcodes/Opcodes.h" +#include "graphics/shader/recompiler/ir/opcodes/ValueOpcodes.h" #include "graphics/shader/shader.h" #include @@ -74,6 +75,24 @@ enum class ResourceKind { Sampler }; +[[nodiscard]] constexpr bool IsAddressResourceKind(ResourceKind kind) { + return kind == ResourceKind::ScalarAddress || kind == ResourceKind::Flat || + kind == ResourceKind::Global || kind == ResourceKind::Scratch; +} + +[[nodiscard]] constexpr bool ImageResourceKindMatches(ResourceKind kind, + ImageResourceClass resource_class) { + switch (resource_class) { + case ImageResourceClass::Sampled: + return kind == ResourceKind::Image || kind == ResourceKind::ImageUint; + case ImageResourceClass::Storage: + return kind == ResourceKind::StorageImage || kind == ResourceKind::StorageImageUint; + case ImageResourceClass::StorageUint: return kind == ResourceKind::StorageImageUint; + case ImageResourceClass::None: return false; + } + return false; +} + struct MemoryInfo { ResourceKind kind = ResourceKind::None; uint32_t resource = 0; @@ -309,6 +328,56 @@ enum class DescriptorBindingKind { Count, }; +[[nodiscard]] inline std::optional +DescriptorBindingForImage(const ImageResource& image) { + using Dimension = Decoder::ImageDimension; + using Kind = DescriptorBindingKind; + + switch (image.kind) { + case ResourceKind::Image: + switch (image.dimension) { + case Dimension::Dim1D: return Kind::Sampled1D; + case Dimension::Dim1DArray: return Kind::Sampled1DArray; + case Dimension::Dim2D: return Kind::Sampled2D; + case Dimension::Dim2DArray: return Kind::Sampled2DArray; + case Dimension::Dim2DMsaa: return Kind::Sampled2DMsaa; + case Dimension::Dim2DMsaaArray: return Kind::Sampled2DMsaaArray; + case Dimension::Dim3D: return Kind::Sampled3D; + default: return std::nullopt; + } + case ResourceKind::ImageUint: + switch (image.dimension) { + case Dimension::Dim1D: return Kind::SampledUint1D; + case Dimension::Dim1DArray: return Kind::SampledUint1DArray; + case Dimension::Dim2D: return Kind::SampledUint2D; + case Dimension::Dim2DArray: return Kind::SampledUint2DArray; + case Dimension::Dim2DMsaa: return Kind::SampledUint2DMsaa; + case Dimension::Dim2DMsaaArray: return Kind::SampledUint2DMsaaArray; + case Dimension::Dim3D: return Kind::SampledUint3D; + default: return std::nullopt; + } + case ResourceKind::StorageImage: + switch (image.dimension) { + case Dimension::Dim1D: return Kind::Storage1D; + case Dimension::Dim1DArray: return Kind::Storage1DArray; + case Dimension::Dim2D: return Kind::Storage2D; + case Dimension::Dim2DArray: return Kind::Storage2DArray; + case Dimension::Dim3D: return Kind::Storage3D; + default: return std::nullopt; + } + case ResourceKind::StorageImageUint: + switch (image.dimension) { + case Dimension::Dim1D: return Kind::StorageUint1D; + case Dimension::Dim1DArray: return Kind::StorageUint1DArray; + case Dimension::Dim2D: return Kind::StorageUint2D; + case Dimension::Dim2DArray: return Kind::StorageUint2DArray; + case Dimension::Dim3D: return Kind::StorageUint3D; + default: return std::nullopt; + } + default: return std::nullopt; + } +} + struct DescriptorBinding { DescriptorBindingKind kind = DescriptorBindingKind::Buffers; uint32_t binding = 0; diff --git a/src/graphics/shader/recompiler/ir/Type.cpp b/src/graphics/shader/recompiler/ir/Type.cpp index ac7e14a2..c0653b72 100644 --- a/src/graphics/shader/recompiler/ir/Type.cpp +++ b/src/graphics/shader/recompiler/ir/Type.cpp @@ -24,8 +24,6 @@ std::string TypeName(Type type) { "U32x3", "U32x4", "F32x2", - "F32x3", - "F32x4", "SrtResource", "BufferResource", "AddressResource", diff --git a/src/graphics/shader/recompiler/ir/Type.h b/src/graphics/shader/recompiler/ir/Type.h index 055b2697..e3a15033 100644 --- a/src/graphics/shader/recompiler/ir/Type.h +++ b/src/graphics/shader/recompiler/ir/Type.h @@ -21,14 +21,12 @@ enum class Type : uint32_t { U32x3 = 1u << 11u, U32x4 = 1u << 12u, F32x2 = 1u << 13u, - F32x3 = 1u << 14u, - F32x4 = 1u << 15u, - SrtResource = 1u << 16u, - BufferResource = 1u << 17u, - AddressResource = 1u << 18u, - ImageResource = 1u << 19u, - SamplerResource = 1u << 20u, - ImageAddress = 1u << 21u, + SrtResource = 1u << 14u, + BufferResource = 1u << 15u, + AddressResource = 1u << 16u, + ImageResource = 1u << 17u, + SamplerResource = 1u << 18u, + ImageAddress = 1u << 19u, }; constexpr Type operator|(Type lhs, Type rhs) { diff --git a/src/graphics/shader/recompiler/ir/ValueProgram.cpp b/src/graphics/shader/recompiler/ir/ValueProgram.cpp index f5df0433..000b1e4c 100644 --- a/src/graphics/shader/recompiler/ir/ValueProgram.cpp +++ b/src/graphics/shader/recompiler/ir/ValueProgram.cpp @@ -44,7 +44,8 @@ bool IsRegisterStatePseudo(ValueOpcode opcode) { } bool IsRuntimeRead(ValueOpcode opcode) { - return opcode == ValueOpcode::LoadAddressU32 || opcode == ValueOpcode::ReadConstBuffer; + return opcode == ValueOpcode::ReadConstBuffer || + AddressOpcodeInfoOf(opcode).access == AddressAccess::Read; } bool EquivalentValue(const ValueProgram& program, Value left, Value right, @@ -324,31 +325,52 @@ bool ValidateValueProgram(const ValueProgram& program, bool require_ssa, std::st ValueOpcodeName(inst.GetOpcode()), inst.NumArgs(), NumArgsOf(inst.GetOpcode()))); } - if (IsRuntimeRead(inst.GetOpcode())) { + if (inst.GetOpcode() == ValueOpcode::ReadConstBuffer) { const auto memory_index = inst.Flags().index; if (memory_index >= program.memory_info.size()) { return Fail(error, fmt::format("{} has an invalid memory-info index", ValueOpcodeName(inst.GetOpcode()))); } - const auto& memory = program.memory_info[memory_index]; - const bool scalar_buffer = inst.GetOpcode() == ValueOpcode::ReadConstBuffer; - const bool scalar_address = memory.kind == ResourceKind::ScalarAddress; - if ((scalar_buffer && memory.kind != ResourceKind::ScalarBuffer) || - (!scalar_buffer && !scalar_address && memory.kind != ResourceKind::Flat && - memory.kind != ResourceKind::Global && memory.kind != ResourceKind::Scratch)) { + const auto& memory = program.memory_info[memory_index]; + if (memory.kind != ResourceKind::ScalarBuffer) { return Fail(error, fmt::format("{} has an invalid scalar-memory resource kind", ValueOpcodeName(inst.GetOpcode()))); } - const bool scalar_memory = scalar_buffer || scalar_address; const bool valid_group_width = - scalar_memory + memory.component_count == 1u || memory.component_count == 2u || + memory.component_count == 4u || memory.component_count == 8u || + memory.component_count == 16u; + if (memory.data_bits != 32u || memory.data_dwords != 1u || !valid_group_width || + memory.component_index >= memory.component_count) { + return Fail(error, fmt::format("{} has inconsistent scalar-memory metadata", + ValueOpcodeName(inst.GetOpcode()))); + } + } + const auto address_info = AddressOpcodeInfoOf(inst.GetOpcode()); + if (address_info.access != AddressAccess::None) { + const auto memory_index = inst.Flags().index; + if (memory_index >= program.memory_info.size()) { + return Fail(error, fmt::format("{} has an invalid memory-info index", + ValueOpcodeName(inst.GetOpcode()))); + } + const auto& memory = program.memory_info[memory_index]; + if (!IsAddressResourceKind(memory.kind) || + (memory.kind == ResourceKind::ScalarAddress && + inst.GetOpcode() != ValueOpcode::LoadAddressU32)) { + return Fail(error, fmt::format("{} has an invalid address resource kind", + ValueOpcodeName(inst.GetOpcode()))); + } + const bool scalar_address = memory.kind == ResourceKind::ScalarAddress; + const bool valid_group_width = + scalar_address ? memory.component_count == 1u || memory.component_count == 2u || memory.component_count == 4u || memory.component_count == 8u || memory.component_count == 16u : memory.component_count >= 1u && memory.component_count <= 4u; - if (memory.data_bits != 32u || memory.data_dwords != 1u || !valid_group_width || - memory.component_index >= memory.component_count) { - return Fail(error, fmt::format("{} has inconsistent scalar-memory metadata", + if (memory.data_bits != address_info.data_bits || memory.data_dwords != 1u || + !valid_group_width || memory.component_index >= memory.component_count || + memory.sampler != 0u) { + return Fail(error, fmt::format("{} has inconsistent address-memory metadata", ValueOpcodeName(inst.GetOpcode()))); } } @@ -407,6 +429,23 @@ bool ValidateValueProgram(const ValueProgram& program, bool require_ssa, std::st ValueOpcodeName(inst.GetOpcode()))); } } + const auto image_info = ImageOpcodeInfoOf(inst.GetOpcode()); + if (image_info.access != ImageAccess::None) { + const auto memory_index = inst.Flags().index; + if (memory_index >= program.memory_info.size()) { + return Fail(error, fmt::format("{} has an invalid memory-info index", + ValueOpcodeName(inst.GetOpcode()))); + } + const auto& memory = program.memory_info[memory_index]; + if (!ImageResourceKindMatches(memory.kind, image_info.resource_class)) { + return Fail(error, fmt::format("{} has invalid image-memory metadata", + ValueOpcodeName(inst.GetOpcode()))); + } + } + if (inst.GetOpcode() == ValueOpcode::SetAttribute && + inst.Flags().index >= program.export_info.size()) { + return Fail(error, "SetAttribute has an invalid export-info index"); + } uint32_t composite_components = 0u; switch (inst.GetOpcode()) { case ValueOpcode::CompositeExtractU64: composite_components = 2u; break; diff --git a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp index 86ae86ad..17676803 100644 --- a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp +++ b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.cpp @@ -79,19 +79,16 @@ bool HasSideEffects(ValueOpcode opcode) { shared_access == SharedAccess::Append || shared_access == SharedAccess::Consume) { return true; } + if (AddressOpcodeInfoOf(opcode).access == AddressAccess::Write) { + return true; + } + const auto image_info = ImageOpcodeInfoOf(opcode); + if (image_info.access == ImageAccess::Write || image_info.access == ImageAccess::Atomic) { + return true; + } switch (opcode) { case ValueOpcode::Reference: case ValueOpcode::ReferenceU32: - case ValueOpcode::StoreAddressU8: - case ValueOpcode::StoreAddressU16: - case ValueOpcode::StoreAddressU32: - case ValueOpcode::ImageAtomicIAdd32: - case ValueOpcode::ImageAtomicUMin32: - case ValueOpcode::ImageAtomicUMax32: - case ValueOpcode::ImageAtomicAnd32: - case ValueOpcode::ImageAtomicOr32: - case ValueOpcode::ImageAtomicXor32: - case ValueOpcode::ImageWrite: case ValueOpcode::SetAttribute: case ValueOpcode::Barrier: return true; default: return false; @@ -185,4 +182,37 @@ uint32_t SharedComponentCount(ValueOpcode opcode) { } } +AddressOpcodeInfo AddressOpcodeInfoOf(ValueOpcode opcode) { + switch (opcode) { + case ValueOpcode::LoadAddressU8: return {AddressAccess::Read, 8u}; + case ValueOpcode::LoadAddressU16: return {AddressAccess::Read, 16u}; + case ValueOpcode::LoadAddressU32: return {AddressAccess::Read, 32u}; + case ValueOpcode::StoreAddressU8: return {AddressAccess::Write, 8u}; + case ValueOpcode::StoreAddressU16: return {AddressAccess::Write, 16u}; + case ValueOpcode::StoreAddressU32: return {AddressAccess::Write, 32u}; + default: return {}; + } +} + +ImageOpcodeInfo ImageOpcodeInfoOf(ValueOpcode opcode) { + switch (opcode) { + case ValueOpcode::ImageQueryDimensions: + case ValueOpcode::ImageRead: return {ImageAccess::Read, ImageResourceClass::Sampled, false}; + case ValueOpcode::ImageQueryLod: + case ValueOpcode::ImageSampleRaw: + case ValueOpcode::ImageGatherRaw: + return {ImageAccess::Read, ImageResourceClass::Sampled, true}; + case ValueOpcode::ImageWrite: + return {ImageAccess::Write, ImageResourceClass::Storage, false}; + case ValueOpcode::ImageAtomicIAdd32: + case ValueOpcode::ImageAtomicUMin32: + case ValueOpcode::ImageAtomicUMax32: + case ValueOpcode::ImageAtomicAnd32: + case ValueOpcode::ImageAtomicOr32: + case ValueOpcode::ImageAtomicXor32: + return {ImageAccess::Atomic, ImageResourceClass::StorageUint, false}; + default: return {}; + } +} + } // namespace Libs::Graphics::ShaderRecompiler::IR diff --git a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.h b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.h index 5636e0d1..c6045f8a 100644 --- a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.h +++ b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.h @@ -18,6 +18,20 @@ enum class ValueOpcode { enum class BufferAccess { None, Read, Write, Atomic }; enum class SharedAccess { None, Read, Write, Atomic, Append, Consume }; +enum class AddressAccess { None, Read, Write }; +enum class ImageAccess { None, Read, Write, Atomic }; +enum class ImageResourceClass { None, Sampled, Storage, StorageUint }; + +struct AddressOpcodeInfo { + AddressAccess access = AddressAccess::None; + uint32_t data_bits = 0; +}; + +struct ImageOpcodeInfo { + ImageAccess access = ImageAccess::None; + ImageResourceClass resource_class = ImageResourceClass::None; + bool needs_sampler = false; +}; struct DppMoveFlags { uint16_t control = 0; @@ -51,14 +65,16 @@ struct ExportFlags { static_assert(sizeof(ExportFlags) == sizeof(uint64_t)); static_assert(std::is_trivially_copyable_v); -[[nodiscard]] Type TypeOf(ValueOpcode opcode); -[[nodiscard]] Type ArgTypeOf(ValueOpcode opcode, size_t index); -[[nodiscard]] size_t NumArgsOf(ValueOpcode opcode); -[[nodiscard]] bool HasSideEffects(ValueOpcode opcode); -[[nodiscard]] BufferAccess BufferAccessOf(ValueOpcode opcode); -[[nodiscard]] uint32_t BufferComponentCount(ValueOpcode opcode); -[[nodiscard]] SharedAccess SharedAccessOf(ValueOpcode opcode); -[[nodiscard]] uint32_t SharedComponentCount(ValueOpcode opcode); -[[nodiscard]] std::string_view ValueOpcodeName(ValueOpcode opcode); +[[nodiscard]] Type TypeOf(ValueOpcode opcode); +[[nodiscard]] Type ArgTypeOf(ValueOpcode opcode, size_t index); +[[nodiscard]] size_t NumArgsOf(ValueOpcode opcode); +[[nodiscard]] bool HasSideEffects(ValueOpcode opcode); +[[nodiscard]] BufferAccess BufferAccessOf(ValueOpcode opcode); +[[nodiscard]] uint32_t BufferComponentCount(ValueOpcode opcode); +[[nodiscard]] SharedAccess SharedAccessOf(ValueOpcode opcode); +[[nodiscard]] uint32_t SharedComponentCount(ValueOpcode opcode); +[[nodiscard]] AddressOpcodeInfo AddressOpcodeInfoOf(ValueOpcode opcode); +[[nodiscard]] ImageOpcodeInfo ImageOpcodeInfoOf(ValueOpcode opcode); +[[nodiscard]] std::string_view ValueOpcodeName(ValueOpcode opcode); } // namespace Libs::Graphics::ShaderRecompiler::IR diff --git a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc index 9471a02b..fa5694e7 100644 --- a/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc +++ b/src/graphics/shader/recompiler/ir/opcodes/ValueOpcodes.inc @@ -2,7 +2,6 @@ VALUE_OPCODE(Phi, Opaque) VALUE_OPCODE(Identity, Opaque, Opaque) VALUE_OPCODE(Void, Void) -VALUE_OPCODE(ConditionRef, U1, U1) VALUE_OPCODE(Reference, Void, U1) VALUE_OPCODE(ReferenceU32, Void, U32) @@ -112,7 +111,6 @@ VALUE_OPCODE(BitCount32, U32, U32) VALUE_OPCODE(BitCount64, U32, U64) VALUE_OPCODE(FindUMsb32, U32, U32) VALUE_OPCODE(FindUMsb64, U32, U64) -VALUE_OPCODE(FindSMsb32, U32, U32) VALUE_OPCODE(FindILsb32, U32, U32) VALUE_OPCODE(SMin32, U32, U32, U32) VALUE_OPCODE(UMin32, U32, U32, U32) @@ -131,19 +129,14 @@ VALUE_OPCODE(ULessThan64, U1, U64, U64) VALUE_OPCODE(IEqual32, U1, U32, U32) VALUE_OPCODE(IEqual64, U1, U64, U64) VALUE_OPCODE(SLessThanEqual32, U1, U32, U32) -VALUE_OPCODE(SLessThanEqual64, U1, U64, U64) VALUE_OPCODE(ULessThanEqual32, U1, U32, U32) -VALUE_OPCODE(ULessThanEqual64, U1, U64, U64) VALUE_OPCODE(SGreaterThan32, U1, U32, U32) -VALUE_OPCODE(SGreaterThan64, U1, U64, U64) VALUE_OPCODE(UGreaterThan32, U1, U32, U32) VALUE_OPCODE(UGreaterThan64, U1, U64, U64) VALUE_OPCODE(INotEqual32, U1, U32, U32) VALUE_OPCODE(INotEqual64, U1, U64, U64) VALUE_OPCODE(SGreaterThanEqual32, U1, U32, U32) -VALUE_OPCODE(SGreaterThanEqual64, U1, U64, U64) VALUE_OPCODE(UGreaterThanEqual32, U1, U32, U32) -VALUE_OPCODE(UGreaterThanEqual64, U1, U64, U64) VALUE_OPCODE(LogicalOr, U1, U1, U1) VALUE_OPCODE(LogicalAnd, U1, U1, U1) VALUE_OPCODE(LogicalXor, U1, U1, U1) diff --git a/src/graphics/shader/recompiler/ir/passes/BindingLayout.cpp b/src/graphics/shader/recompiler/ir/passes/BindingLayout.cpp index 7cb41bf6..69c37300 100644 --- a/src/graphics/shader/recompiler/ir/passes/BindingLayout.cpp +++ b/src/graphics/shader/recompiler/ir/passes/BindingLayout.cpp @@ -39,55 +39,6 @@ constexpr std::array ImageBindingKinds = { DescriptorBindingKind::StorageUint3D, }; -bool ImageBinding(const ImageResource& image, DescriptorBindingKind& result) { - using Dimension = Decoder::ImageDimension; - using Kind = DescriptorBindingKind; - - switch (image.kind) { - case ResourceKind::Image: - switch (image.dimension) { - case Dimension::Dim1D: result = Kind::Sampled1D; return true; - case Dimension::Dim1DArray: result = Kind::Sampled1DArray; return true; - case Dimension::Dim2D: result = Kind::Sampled2D; return true; - case Dimension::Dim2DArray: result = Kind::Sampled2DArray; return true; - case Dimension::Dim2DMsaa: result = Kind::Sampled2DMsaa; return true; - case Dimension::Dim2DMsaaArray: result = Kind::Sampled2DMsaaArray; return true; - case Dimension::Dim3D: result = Kind::Sampled3D; return true; - default: return false; - } - case ResourceKind::ImageUint: - switch (image.dimension) { - case Dimension::Dim1D: result = Kind::SampledUint1D; return true; - case Dimension::Dim1DArray: result = Kind::SampledUint1DArray; return true; - case Dimension::Dim2D: result = Kind::SampledUint2D; return true; - case Dimension::Dim2DArray: result = Kind::SampledUint2DArray; return true; - case Dimension::Dim2DMsaa: result = Kind::SampledUint2DMsaa; return true; - case Dimension::Dim2DMsaaArray: result = Kind::SampledUint2DMsaaArray; return true; - case Dimension::Dim3D: result = Kind::SampledUint3D; return true; - default: return false; - } - case ResourceKind::StorageImage: - switch (image.dimension) { - case Dimension::Dim1D: result = Kind::Storage1D; return true; - case Dimension::Dim1DArray: result = Kind::Storage1DArray; return true; - case Dimension::Dim2D: result = Kind::Storage2D; return true; - case Dimension::Dim2DArray: result = Kind::Storage2DArray; return true; - case Dimension::Dim3D: result = Kind::Storage3D; return true; - default: return false; - } - case ResourceKind::StorageImageUint: - switch (image.dimension) { - case Dimension::Dim1D: result = Kind::StorageUint1D; return true; - case Dimension::Dim1DArray: result = Kind::StorageUint1DArray; return true; - case Dimension::Dim2D: result = Kind::StorageUint2D; return true; - case Dimension::Dim2DArray: result = Kind::StorageUint2DArray; return true; - case Dimension::Dim3D: result = Kind::StorageUint3D; return true; - default: return false; - } - default: return false; - } -} - bool CollectUserData(const Program& program, std::vector& result) { std::set registers; for (const auto* block: program.values->blocks) { @@ -187,14 +138,14 @@ bool AllocateBindings(Program& program, const BindingLayoutOptions& options, std std::array, ImageBindingKinds.size()> image_groups; for (uint32_t i = 0; i < program.info.images.size(); i++) { - DescriptorBindingKind kind; - if (!ImageBinding(program.info.images[i], kind)) { + const auto kind = DescriptorBindingForImage(program.info.images[i]); + if (!kind.has_value()) { if (error != nullptr) { *error = "shader info contains an invalid image binding class"; } return false; } - const auto group = std::find(ImageBindingKinds.begin(), ImageBindingKinds.end(), kind); + const auto group = std::find(ImageBindingKinds.begin(), ImageBindingKinds.end(), *kind); if (group == ImageBindingKinds.end()) { if (error != nullptr) { *error = "shader info contains an unmapped image binding class"; diff --git a/src/graphics/shader/recompiler/ir/passes/ConstantPropagation.cpp b/src/graphics/shader/recompiler/ir/passes/ConstantPropagation.cpp index 42cde88a..21bc6ac9 100644 --- a/src/graphics/shader/recompiler/ir/passes/ConstantPropagation.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ConstantPropagation.cpp @@ -525,28 +525,12 @@ void FoldInstruction(Inst& inst) { case ValueOpcode::ULessThan64: FoldU64Compare(inst, [](uint64_t a, uint64_t b) { return a < b; }); return; - case ValueOpcode::ULessThanEqual64: - FoldU64Compare(inst, [](uint64_t a, uint64_t b) { return a <= b; }); - return; case ValueOpcode::UGreaterThan64: FoldU64Compare(inst, [](uint64_t a, uint64_t b) { return a > b; }); return; - case ValueOpcode::UGreaterThanEqual64: - FoldU64Compare(inst, [](uint64_t a, uint64_t b) { return a >= b; }); - return; case ValueOpcode::SLessThan64: - case ValueOpcode::SLessThanEqual64: - case ValueOpcode::SGreaterThan64: - case ValueOpcode::SGreaterThanEqual64: - FoldU64Compare(inst, [opcode = inst.GetOpcode()](uint64_t a, uint64_t b) { - const auto lhs = std::bit_cast(a); - const auto rhs = std::bit_cast(b); - switch (opcode) { - case ValueOpcode::SLessThan64: return lhs < rhs; - case ValueOpcode::SLessThanEqual64: return lhs <= rhs; - case ValueOpcode::SGreaterThan64: return lhs > rhs; - default: return lhs >= rhs; - } + FoldU64Compare(inst, [](uint64_t a, uint64_t b) { + return std::bit_cast(a) < std::bit_cast(b); }); return; case ValueOpcode::LogicalAnd: diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp index 179e8199..a915c21e 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceMaterialization.cpp @@ -264,24 +264,6 @@ uint64_t AddressSpecialization(const AddressResource& resource, : snapshot.guest_base - snapshot.binding_base; } -bool IsImageOp(ValueOpcode op) { - switch (op) { - case ValueOpcode::ImageQueryDimensions: - case ValueOpcode::ImageQueryLod: - case ValueOpcode::ImageRead: - case ValueOpcode::ImageWrite: - case ValueOpcode::ImageSampleRaw: - case ValueOpcode::ImageGatherRaw: - case ValueOpcode::ImageAtomicIAdd32: - case ValueOpcode::ImageAtomicUMin32: - case ValueOpcode::ImageAtomicUMax32: - case ValueOpcode::ImageAtomicAnd32: - case ValueOpcode::ImageAtomicOr32: - case ValueOpcode::ImageAtomicXor32: return true; - default: return false; - } -} - size_t FlattenedRuntimeDwords(const Program& program) { size_t size = program.values != nullptr ? program.values->srt_reads.size() : 0u; for (uint32_t resource = 0; resource < program.info.images.size(); resource++) { @@ -915,7 +897,7 @@ bool SpecializeResources(Program& program, ResourceSnapshot& snapshot, std::stri auto memory_info = program.values->memory_info; for (const auto* block: program.values->blocks) { for (const auto& inst: *block) { - if (!IsImageOp(inst.GetOpcode())) { + if (ImageOpcodeInfoOf(inst.GetOpcode()).access == ImageAccess::None) { continue; } const auto index = inst.Flags().index; diff --git a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp index f137a1a4..89e6d5aa 100644 --- a/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp +++ b/src/graphics/shader/recompiler/ir/passes/ResourceTracking.cpp @@ -25,60 +25,6 @@ const char* StageName(ShaderType stage) { } } -bool IsAddressStore(ValueOpcode op) { - switch (op) { - case ValueOpcode::StoreAddressU8: - case ValueOpcode::StoreAddressU16: - case ValueOpcode::StoreAddressU32: return true; - default: return false; - } -} - -bool IsAddress(ValueOpcode op) { - switch (op) { - case ValueOpcode::LoadAddressU8: - case ValueOpcode::LoadAddressU16: - case ValueOpcode::LoadAddressU32: - case ValueOpcode::StoreAddressU8: - case ValueOpcode::StoreAddressU16: - case ValueOpcode::StoreAddressU32: return true; - default: return false; - } -} - -bool IsImageAtomic(ValueOpcode op) { - switch (op) { - case ValueOpcode::ImageAtomicIAdd32: - case ValueOpcode::ImageAtomicUMin32: - case ValueOpcode::ImageAtomicUMax32: - case ValueOpcode::ImageAtomicAnd32: - case ValueOpcode::ImageAtomicOr32: - case ValueOpcode::ImageAtomicXor32: return true; - default: return false; - } -} - -bool IsImageStore(ValueOpcode op) { - return op == ValueOpcode::ImageWrite || IsImageAtomic(op); -} - -bool IsImage(ValueOpcode op) { - switch (op) { - case ValueOpcode::ImageQueryDimensions: - case ValueOpcode::ImageQueryLod: - case ValueOpcode::ImageRead: - case ValueOpcode::ImageWrite: - case ValueOpcode::ImageSampleRaw: - case ValueOpcode::ImageGatherRaw: return true; - default: return IsImageAtomic(op); - } -} - -bool NeedsSampler(ValueOpcode op) { - return op == ValueOpcode::ImageQueryLod || op == ValueOpcode::ImageSampleRaw || - op == ValueOpcode::ImageGatherRaw; -} - uint32_t ByteExtent(const MemoryInfo& memory) { const auto bytes = std::max((memory.data_bits + 7u) / 8u, 1u); const auto count = std::max(memory.data_dwords, 1u); @@ -353,7 +299,9 @@ private: for (const auto* block: m_values.blocks) { for (const auto& inst: *block) { const auto op = inst.GetOpcode(); - if ((BufferAccessOf(op) == BufferAccess::None && !IsAddress(op) && !IsImage(op)) || + if ((BufferAccessOf(op) == BufferAccess::None && + AddressOpcodeInfoOf(op).access == AddressAccess::None && + ImageOpcodeInfoOf(op).access == ImageAccess::None) || &inst == &owner) { continue; } @@ -534,7 +482,8 @@ private: void PlanIndirectImages() { for (auto* block: m_values.blocks) { for (auto& inst: *block) { - if (!IsImage(inst.GetOpcode()) || inst.NumArgs() == 0u) { + if (ImageOpcodeInfoOf(inst.GetOpcode()).access == ImageAccess::None || + inst.NumArgs() == 0u) { continue; } auto* handle = inst.Arg(0).Resolve().TryInstruction(); @@ -671,8 +620,9 @@ private: } static void Merge(ImageResource& image, ValueOpcode op, uint32_t pc) { - const bool atomic = IsImageAtomic(op); - const bool write = IsImageStore(op); + const auto access = ImageOpcodeInfoOf(op).access; + const bool atomic = access == ImageAccess::Atomic; + const bool write = access == ImageAccess::Write || atomic; image.first_use_pc = std::min(image.first_use_pc, pc); image.read = image.read || !write || atomic; image.written = image.written || write; @@ -693,8 +643,8 @@ private: return static_cast(m_info.samplers.size() - 1); } - uint32_t AddAddress(uint32_t source, bool unbased, const MemoryInfo& memory, ValueOpcode op, - uint32_t pc) { + uint32_t AddAddress(uint32_t source, bool unbased, const MemoryInfo& memory, + AddressAccess access, uint32_t pc) { auto immediate = static_cast(memory.offset); if (memory.kind == ResourceKind::ScalarAddress) { immediate = static_cast(static_cast(immediate) & ~3u); @@ -706,8 +656,8 @@ private: address.kind == memory.kind) { address.first_use_pc = std::min(address.first_use_pc, pc); address.min_offset = std::min(address.min_offset, min_offset); - address.read = address.read || !IsAddressStore(op); - address.written = address.written || IsAddressStore(op); + address.read = address.read || access == AddressAccess::Read; + address.written = address.written || access == AddressAccess::Write; return i; } } @@ -720,8 +670,8 @@ private: address.kind = memory.kind; address.min_offset = min_offset; address.unbased = unbased; - address.read = !IsAddressStore(op); - address.written = IsAddressStore(op); + address.read = access == AddressAccess::Read; + address.written = access == AddressAccess::Write; m_info.addresses.push_back(address); return static_cast(m_info.addresses.size() - 1); } @@ -774,8 +724,12 @@ private: } bool Collect(Inst& inst, std::string* error) { - const auto op = inst.GetOpcode(); - if (BufferAccessOf(op) == BufferAccess::None && !IsAddress(op) && !IsImage(op)) { + const auto op = inst.GetOpcode(); + const auto buffer = BufferAccessOf(op); + const auto address_info = AddressOpcodeInfoOf(op); + const auto image_info = ImageOpcodeInfoOf(op); + if (buffer == BufferAccess::None && address_info.access == AddressAccess::None && + image_info.access == ImageAccess::None) { return true; } const auto flags = inst.Flags(); @@ -794,7 +748,7 @@ private: uint32_t source = 0; uint32_t resource = 0; - if (BufferAccessOf(op) != BufferAccess::None) { + if (buffer != BufferAccess::None) { if (!GetHandle(inst.Arg(0), ValueOpcode::GetBufferResource, 4, flags.pc, handle, source, error)) { return false; @@ -806,12 +760,15 @@ private: return AddHandlePatch(handle, resource, flags.pc, error) && AddMemoryPatch(flags.index, resource, 0, false, flags.pc, error); } - if (IsAddress(op)) { + if (address_info.access != AddressAccess::None) { + if (!IsAddressResourceKind(memory.kind)) { + return Fail(flags.pc, error, "address operation has invalid resource kind"); + } bool unbased = false; if (!GetAddressHandle(inst, inst.Arg(0), flags.pc, handle, source, unbased, error)) { return false; } - resource = AddAddress(source, unbased, memory, op, flags.pc); + resource = AddAddress(source, unbased, memory, address_info.access, flags.pc); if (resource == UINT32_MAX) { return Fail(flags.pc, error, "address resource limit exceeded"); } @@ -819,6 +776,9 @@ private: AddMemoryPatch(flags.index, resource, 0, false, flags.pc, error); } + if (!ImageResourceKindMatches(memory.kind, image_info.resource_class)) { + return Fail(flags.pc, error, "image operation has invalid resource kind"); + } handle = inst.Arg(0).Resolve().TryInstruction(); const auto* indirect = handle != nullptr ? FindIndirectImage(*handle) : nullptr; if (indirect != nullptr) { @@ -835,7 +795,7 @@ private: return false; } uint32_t sampler = 0; - if (NeedsSampler(op)) { + if (image_info.needs_sampler) { if (inst.NumArgs() < 2) { return Fail(flags.pc, error, "sampled image operation has no sampler handle"); } @@ -854,7 +814,8 @@ private: return false; } } - return AddMemoryPatch(flags.index, resource, sampler, NeedsSampler(op), flags.pc, error); + return AddMemoryPatch(flags.index, resource, sampler, image_info.needs_sampler, flags.pc, + error); } const DescriptorSource* Source(uint32_t source) const { diff --git a/tests/ResourceTrackingTests.cpp b/tests/ResourceTrackingTests.cpp index e4fa1ed1..7776fe3e 100644 --- a/tests/ResourceTrackingTests.cpp +++ b/tests/ResourceTrackingTests.cpp @@ -597,7 +597,7 @@ void TestImagesSamplersAndAliases() { const auto storage = fixture.Image(image_words, 16); MemoryInfo storage_memory; - storage_memory.kind = ResourceKind::StorageImage; + storage_memory.kind = ResourceKind::StorageImageUint; storage_memory.image_dimension = Decoder::ImageDimension::Dim2D; fixture.Emit(ValueOpcode::ImageAtomicIAdd32, {storage, image_address, Value(1u), Value(true)}, @@ -1146,6 +1146,79 @@ void TestResourceLimitIsTransactional() { "resource-limit failure partially mutated typed resource state"); } +void TestMalformedMemoryKindsRejected() { + { + Fixture fixture; + const auto address = fixture.Address(Value(0u), Value(0u), 4); + MemoryInfo memory; + memory.kind = ResourceKind::Buffer; + fixture.Emit(ValueOpcode::StoreAddressU32, + {address, Value(0u), Value(0u), Value(1u), Value(true)}, + fixture.AddMemory(memory, 4)); + std::string error; + Check(BuildSrtPlan(fixture.program, &error) && + !TrackResources(fixture.program, &error) && + error.find("address operation has invalid resource kind") != + std::string::npos, + "resource tracking accepted an address opcode with buffer metadata"); + } + { + Fixture fixture; + const auto image = + fixture.Image({Value(0u), Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u)}, + 8); + MemoryInfo memory; + memory.kind = ResourceKind::Flat; + fixture.Emit(ValueOpcode::ImageRead, + {image, fixture.ImageAddress(), Value(true)}, + fixture.AddMemory(memory, 8)); + std::string error; + Check(BuildSrtPlan(fixture.program, &error) && + !TrackResources(fixture.program, &error) && + error.find("image operation has invalid resource kind") != + std::string::npos, + "resource tracking accepted an image opcode with address metadata"); + } + { + Fixture fixture; + const auto image = + fixture.Image({Value(0u), Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u)}, + 12); + MemoryInfo memory; + memory.kind = ResourceKind::StorageImage; + fixture.Emit(ValueOpcode::ImageRead, + {image, fixture.ImageAddress(), Value(true)}, + fixture.AddMemory(memory, 12)); + std::string error; + Check(BuildSrtPlan(fixture.program, &error) && + !TrackResources(fixture.program, &error) && + error.find("image operation has invalid resource kind") != + std::string::npos, + "resource tracking accepted a sampled read with storage metadata"); + } + { + Fixture fixture; + const auto image = + fixture.Image({Value(0u), Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u)}, + 16); + MemoryInfo memory; + memory.kind = ResourceKind::StorageImage; + fixture.Emit(ValueOpcode::ImageAtomicIAdd32, + {image, fixture.ImageAddress(), Value(1u), Value(true)}, + fixture.AddMemory(memory, 16)); + std::string error; + Check( + BuildSrtPlan(fixture.program, &error) && + !TrackResources(fixture.program, &error) && + error.find("image operation has invalid resource kind") != + std::string::npos, + "resource tracking accepted a uint atomic with float storage metadata"); + } +} + } // namespace int main() { @@ -1173,6 +1246,7 @@ int main() { Run("buffer swizzle specialization", TestBufferSwizzleSpecialization); Run("shader info and bindings", TestShaderInfoAndBindingLayout); Run("resource limit", TestResourceLimitIsTransactional); + Run("malformed memory kinds", TestMalformedMemoryKindsRejected); } catch (const std::exception &exception) { std::cerr << "resource tracking test failed: " << exception.what() << '\n'; return 1; diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index d4e5dfa0..df96d382 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -9118,7 +9118,7 @@ void TestNativeWideValueValidation() { memory.data_bits = 16; program.memory_info.push_back(memory); append_scalar_read(program, ValueOpcode::LoadAddressU32, {}); - check_rejected(program, "inconsistent scalar-memory metadata"); + check_rejected(program, "inconsistent address-memory metadata"); } { auto program = make_program(); @@ -9146,7 +9146,7 @@ void TestNativeWideValueValidation() { memory.component_index = 4; program.memory_info.push_back(memory); append_scalar_read(program, ValueOpcode::LoadAddressU32, {}); - check_rejected(program, "inconsistent scalar-memory metadata"); + check_rejected(program, "inconsistent address-memory metadata"); } { @@ -9248,6 +9248,131 @@ void TestNativeWideValueValidation() { append_shared(program, ValueOpcode::LoadSharedU32x3, {}); check_rejected(program, "inconsistent shared-memory width"); } + const auto append_address = [](ValueProgram &program, ValueOpcode opcode, + MemoryFlags flags) { + IREmitter ir(program.blocks.front()); + const auto resource = + ir.Emit(ValueOpcode::GetAddressResource, {Value(0u), Value(0u)}); + if (opcode == ValueOpcode::StoreAddressU32) { + ir.Emit(opcode, {resource, Value(0u), Value(0u), Value(0u), Value(true)}, + flags); + } else { + ir.Emit(opcode, {resource, Value(0u), Value(0u), Value(true)}, flags); + } + }; + { + auto program = make_program(); + append_address(program, ValueOpcode::StoreAddressU32, + MemoryFlags{.index = 1}); + check_rejected(program, "invalid memory-info index"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::Buffer; + program.memory_info.push_back(memory); + append_address(program, ValueOpcode::StoreAddressU32, {}); + check_rejected(program, "invalid address resource kind"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::Global; + program.memory_info.push_back(memory); + append_address(program, ValueOpcode::LoadAddressU8, {}); + check_rejected(program, "inconsistent address-memory metadata"); + } + const auto append_image = [](ValueProgram &program, ValueOpcode opcode, + MemoryFlags flags) { + IREmitter ir(program.blocks.front()); + const auto image = ir.Emit(ValueOpcode::GetImageResource, + {Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u), Value(0u)}); + const auto address = + ir.Emit(ValueOpcode::MakeImageAddress, + {Value(0u), Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u), Value(0u), Value(0u), + Value(0u), Value(0u), Value(0u)}); + const auto image_info = ImageOpcodeInfoOf(opcode); + if (opcode == ValueOpcode::ImageWrite) { + const auto data = ir.Emit(ValueOpcode::CompositeConstructU32x4, + {Value(0u), Value(0u), Value(0u), Value(0u)}); + ir.Emit(opcode, {image, address, data, Value(true)}, flags); + } else if (image_info.access == ImageAccess::Atomic) { + ir.Emit(opcode, {image, address, Value(0u), Value(true)}, flags); + } else if (image_info.needs_sampler) { + const auto sampler = + ir.Emit(ValueOpcode::GetSamplerResource, + {Value(0u), Value(0u), Value(0u), Value(0u)}); + ir.Emit(opcode, {image, sampler, address}, flags); + } else if (opcode == ValueOpcode::ImageQueryDimensions) { + ir.Emit(opcode, {image, address}, flags); + } else { + ir.Emit(opcode, {image, address, Value(true)}, flags); + } + }; + { + auto program = make_program(); + append_image(program, ValueOpcode::ImageRead, MemoryFlags{.index = 1}); + check_rejected(program, "invalid memory-info index"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::Buffer; + program.memory_info.push_back(memory); + append_image(program, ValueOpcode::ImageRead, {}); + check_rejected(program, "invalid image-memory metadata"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::StorageImage; + program.memory_info.push_back(memory); + append_image(program, ValueOpcode::ImageRead, {}); + check_rejected(program, "invalid image-memory metadata"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::StorageImageUint; + program.memory_info.push_back(memory); + append_image(program, ValueOpcode::ImageQueryDimensions, {}); + check_rejected(program, "invalid image-memory metadata"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::Image; + program.memory_info.push_back(memory); + append_image(program, ValueOpcode::ImageWrite, {}); + check_rejected(program, "invalid image-memory metadata"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::StorageImage; + program.memory_info.push_back(memory); + append_image(program, ValueOpcode::ImageSampleRaw, {}); + check_rejected(program, "invalid image-memory metadata"); + } + { + auto program = make_program(); + MemoryInfo memory; + memory.kind = ResourceKind::StorageImage; + program.memory_info.push_back(memory); + append_image(program, ValueOpcode::ImageAtomicIAdd32, {}); + check_rejected(program, "invalid image-memory metadata"); + } + { + auto program = make_program(); + IREmitter ir(program.blocks.front()); + const auto data = ir.Emit(ValueOpcode::CompositeConstructU32x4, + {Value(0u), Value(0u), Value(0u), Value(0u)}); + ir.Emit(ValueOpcode::SetAttribute, {data, Value(true)}, + ExportFlags{.index = 1}); + check_rejected(program, "invalid export-info index"); + } } void TestNewShaderRecompilerZeroInitialRegisterState() {