From 103b7312d529f0fba58b82f6a295b1b91255d295 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 17 Aug 2023 22:39:56 +0300 Subject: [PATCH 1/4] tests: don't pass any `-C target-feature=...` for tests by default. --- crates/spirv-std/src/arch/primitive.rs | 2 ++ tests/compiletests/src/main.rs | 3 --- .../arch/convert_u_to_acceleration_structure_khr.rs | 2 +- tests/compiletests/ui/arch/emit_stream_vertex.rs | 2 +- tests/compiletests/ui/arch/end_stream_primitive.rs | 2 +- tests/compiletests/ui/arch/integer_min_and_max.rs | 1 + .../ui/dis/const-float-cast-optimized.rs | 2 +- tests/compiletests/ui/dis/const-float-cast.rs | 2 +- tests/compiletests/ui/dis/const-narrowing-cast.rs | 2 +- tests/compiletests/ui/dis/pass-mode-cast-struct.rs | 2 +- tests/compiletests/ui/image/image_with.rs | 13 +++++++++++-- .../compiletests/ui/lang/control_flow/issue_764.rs | 8 ++++---- .../ui/lang/core/array/init_array_i16.rs | 1 + .../ui/lang/core/array/init_array_i64.rs | 1 + .../ui/lang/core/array/init_array_i8.rs | 1 + tests/compiletests/ui/lang/core/intrinsics/bswap.rs | 1 + .../ui/lang/core/intrinsics/leading_zeros.rs | 1 + .../ui/lang/core/intrinsics/trailing_zeros.rs | 1 + .../ui/lang/core/ptr/allocate_const_scalar.rs | 4 ++-- .../ui/lang/core/ptr/allocate_const_scalar.stderr | 2 +- tests/compiletests/ui/lang/u32/bit_reverse.rs | 1 + tests/compiletests/ui/lang/u32/count_ones.rs | 1 + 22 files changed, 36 insertions(+), 19 deletions(-) diff --git a/crates/spirv-std/src/arch/primitive.rs b/crates/spirv-std/src/arch/primitive.rs index 5953c99426..c8d108522a 100644 --- a/crates/spirv-std/src/arch/primitive.rs +++ b/crates/spirv-std/src/arch/primitive.rs @@ -47,6 +47,7 @@ pub unsafe fn end_primitive() { #[spirv_std_macros::gpu_only] #[doc(alias = "OpEmitStreamVertex")] #[inline] +// FIXME(eddyb) why does this require `i64` instead of `i32`? pub unsafe fn emit_stream_vertex() { unsafe { asm! { @@ -69,6 +70,7 @@ pub unsafe fn emit_stream_vertex() { #[spirv_std_macros::gpu_only] #[doc(alias = "OpEndStreamPrimitive")] #[inline] +// FIXME(eddyb) why does this require `i64` instead of `i32`? pub unsafe fn end_stream_primitive() { unsafe { asm! { diff --git a/tests/compiletests/src/main.rs b/tests/compiletests/src/main.rs index 5cc44f76a5..c4bb687700 100644 --- a/tests/compiletests/src/main.rs +++ b/tests/compiletests/src/main.rs @@ -355,8 +355,6 @@ struct TestDeps { /// The RUSTFLAGS passed to all SPIR-V builds. // FIXME(eddyb) expose most of these from `spirv-builder`. fn rust_flags(codegen_backend_path: &Path) -> String { - let target_features = ["Int8", "Int16", "Int64", "Float64"]; - [ &*format!("-Zcodegen-backend={}", codegen_backend_path.display()), // Ensure the codegen backend is emitted in `.d` files to force Cargo @@ -388,7 +386,6 @@ fn rust_flags(codegen_backend_path: &Path) -> String { // NOTE(eddyb) flags copied from `spirv-builder` are all above this line. "-Cdebuginfo=2", "-Cembed-bitcode=no", - &format!("-Ctarget-feature=+{}", target_features.join(",+")), ] .join(" ") } diff --git a/tests/compiletests/ui/arch/convert_u_to_acceleration_structure_khr.rs b/tests/compiletests/ui/arch/convert_u_to_acceleration_structure_khr.rs index b053ef6d87..1a38c77980 100644 --- a/tests/compiletests/ui/arch/convert_u_to_acceleration_structure_khr.rs +++ b/tests/compiletests/ui/arch/convert_u_to_acceleration_structure_khr.rs @@ -1,5 +1,5 @@ // build-pass -// compile-flags: -Ctarget-feature=+RayTracingKHR,+ext:SPV_KHR_ray_tracing +// compile-flags: -Ctarget-feature=+Int64,+RayTracingKHR,+ext:SPV_KHR_ray_tracing use spirv_std::spirv; diff --git a/tests/compiletests/ui/arch/emit_stream_vertex.rs b/tests/compiletests/ui/arch/emit_stream_vertex.rs index 09b5874f61..3f605a0d53 100644 --- a/tests/compiletests/ui/arch/emit_stream_vertex.rs +++ b/tests/compiletests/ui/arch/emit_stream_vertex.rs @@ -1,5 +1,5 @@ // build-pass -// compile-flags: -C target-feature=+GeometryStreams +// compile-flags: -C target-feature=+Int64,+GeometryStreams use spirv_std::spirv; diff --git a/tests/compiletests/ui/arch/end_stream_primitive.rs b/tests/compiletests/ui/arch/end_stream_primitive.rs index 82e138538d..abe041e1a7 100644 --- a/tests/compiletests/ui/arch/end_stream_primitive.rs +++ b/tests/compiletests/ui/arch/end_stream_primitive.rs @@ -1,5 +1,5 @@ // build-pass -// compile-flags: -C target-feature=+GeometryStreams +// compile-flags: -C target-feature=+Int64,+GeometryStreams use spirv_std::spirv; diff --git a/tests/compiletests/ui/arch/integer_min_and_max.rs b/tests/compiletests/ui/arch/integer_min_and_max.rs index 3adeb183ab..88a21c8f54 100644 --- a/tests/compiletests/ui/arch/integer_min_and_max.rs +++ b/tests/compiletests/ui/arch/integer_min_and_max.rs @@ -1,4 +1,5 @@ // build-pass +// compile-flags: -C target-feature=+Int8,+Int16,+Int64 use spirv_std::arch::{signed_max, signed_min, unsigned_max, unsigned_min}; use spirv_std::spirv; diff --git a/tests/compiletests/ui/dis/const-float-cast-optimized.rs b/tests/compiletests/ui/dis/const-float-cast-optimized.rs index 72b103fde8..428e0ad36f 100644 --- a/tests/compiletests/ui/dis/const-float-cast-optimized.rs +++ b/tests/compiletests/ui/dis/const-float-cast-optimized.rs @@ -2,7 +2,7 @@ // the smaller float type when not needed elsewhere. // build-pass -// compile-flags: -C llvm-args=--disassemble-globals +// compile-flags: -C target-feature=+Float64 -C llvm-args=--disassemble-globals // normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> "" // normalize-stderr-test "OpSource .*\n" -> "" // normalize-stderr-test "OpExtension .SPV_KHR_vulkan_memory_model.\n" -> "" diff --git a/tests/compiletests/ui/dis/const-float-cast.rs b/tests/compiletests/ui/dis/const-float-cast.rs index 59be834504..d98210c97e 100644 --- a/tests/compiletests/ui/dis/const-float-cast.rs +++ b/tests/compiletests/ui/dis/const-float-cast.rs @@ -1,7 +1,7 @@ // Test whether float constant casts need optimization // build-pass -// compile-flags: -C llvm-args=--disassemble-globals +// compile-flags: -C target-feature=+Float64 -C llvm-args=--disassemble-globals // normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> "" // normalize-stderr-test "OpSource .*\n" -> "" // normalize-stderr-test "OpExtension .SPV_KHR_vulkan_memory_model.\n" -> "" diff --git a/tests/compiletests/ui/dis/const-narrowing-cast.rs b/tests/compiletests/ui/dis/const-narrowing-cast.rs index 168d642868..c39e683ab0 100644 --- a/tests/compiletests/ui/dis/const-narrowing-cast.rs +++ b/tests/compiletests/ui/dis/const-narrowing-cast.rs @@ -2,7 +2,7 @@ // and produce the expected truncation behavior. // build-pass -// compile-flags: -C llvm-args=--disassemble-globals +// compile-flags: -C target-feature=+Int8 -C llvm-args=--disassemble-globals // normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> "" // normalize-stderr-test "OpSource .*\n" -> "" // normalize-stderr-test "OpExtension .SPV_KHR_vulkan_memory_model.\n" -> "" diff --git a/tests/compiletests/ui/dis/pass-mode-cast-struct.rs b/tests/compiletests/ui/dis/pass-mode-cast-struct.rs index 5ed7442537..10018f2566 100644 --- a/tests/compiletests/ui/dis/pass-mode-cast-struct.rs +++ b/tests/compiletests/ui/dis/pass-mode-cast-struct.rs @@ -5,7 +5,7 @@ // the default Rust ABI adjustments, that we now override through query hooks) // build-pass -// compile-flags: -C llvm-args=--disassemble-entry=main +// compile-flags: -C target-feature=+Int8,+Int64 -C llvm-args=--disassemble-entry=main use spirv_std::spirv; diff --git a/tests/compiletests/ui/image/image_with.rs b/tests/compiletests/ui/image/image_with.rs index 90d8d62b2f..b89e97c590 100644 --- a/tests/compiletests/ui/image/image_with.rs +++ b/tests/compiletests/ui/image/image_with.rs @@ -11,8 +11,17 @@ pub fn main( output: &mut glam::Vec4, ) { let t1 = image1.fetch_with(glam::IVec2::new(0, 0), sample_with::sample_index(1)); - let t2 = image2.sample_with(*sampler, glam::Vec2::new(0.5, 0.5), sample_with::bias(1.0)); - let t3 = image2.sample_with(*sampler, glam::Vec2::new(0.5, 0.5), sample_with::lod(2.0)); + // FIXME(eddyb) get `f32` to be automatically inferred instead of the `f64` default. + let t2 = image2.sample_with( + *sampler, + glam::Vec2::new(0.5, 0.5), + sample_with::bias(1.0f32), + ); + let t3 = image2.sample_with( + *sampler, + glam::Vec2::new(0.5, 0.5), + sample_with::lod(2.0f32), + ); let t4 = image2.sample_with( *sampler, glam::Vec2::new(0.5, 0.5), diff --git a/tests/compiletests/ui/lang/control_flow/issue_764.rs b/tests/compiletests/ui/lang/control_flow/issue_764.rs index a1a315908d..e8f4700ad4 100644 --- a/tests/compiletests/ui/lang/control_flow/issue_764.rs +++ b/tests/compiletests/ui/lang/control_flow/issue_764.rs @@ -5,7 +5,7 @@ use spirv_std::glam; use spirv_std::glam::{Mat3, Vec3, Vec4}; use spirv_std::spirv; -fn index_to_transform(index: usize, raw_data: &[u8]) -> Transform2D { +fn index_to_transform(index: usize, raw_data: &[u32]) -> Transform2D { Transform2D { own_transform: Mat3::IDENTITY, parent_offset: 0, @@ -21,11 +21,11 @@ struct Transform2D { } trait GivesFinalTransform { - fn get_final_transform(&self, raw_data: &[u8]) -> Mat3; + fn get_final_transform(&self, raw_data: &[u32]) -> Mat3; } impl GivesFinalTransform for (i32, Transform2D) { - fn get_final_transform(&self, raw_data: &[u8]) -> Mat3 { + fn get_final_transform(&self, raw_data: &[u32]) -> Mat3 { if self.1.parent_offset == 0 { self.1.own_transform } else { @@ -44,7 +44,7 @@ impl GivesFinalTransform for (i32, Transform2D) { #[spirv(compute(threads(64)))] pub fn main_cs( #[spirv(global_invocation_id)] id: UVec3, - #[spirv(storage_buffer, descriptor_set = 0, binding = 0)] raw_data: &mut [u8], + #[spirv(storage_buffer, descriptor_set = 0, binding = 0)] raw_data: &mut [u32], #[spirv(position)] output_position: &mut Vec4, ) { let index = id.x as usize; diff --git a/tests/compiletests/ui/lang/core/array/init_array_i16.rs b/tests/compiletests/ui/lang/core/array/init_array_i16.rs index d0f3269e0c..78bc073a9a 100644 --- a/tests/compiletests/ui/lang/core/array/init_array_i16.rs +++ b/tests/compiletests/ui/lang/core/array/init_array_i16.rs @@ -1,5 +1,6 @@ // Test creating an array. // build-pass +// compile-flags: -C target-feature=+Int16 use spirv_std::macros::spirv; diff --git a/tests/compiletests/ui/lang/core/array/init_array_i64.rs b/tests/compiletests/ui/lang/core/array/init_array_i64.rs index 8f5120c27c..edad12768e 100644 --- a/tests/compiletests/ui/lang/core/array/init_array_i64.rs +++ b/tests/compiletests/ui/lang/core/array/init_array_i64.rs @@ -1,5 +1,6 @@ // Test creating an array. // build-pass +// compile-flags: -C target-feature=+Int64 use spirv_std::macros::spirv; diff --git a/tests/compiletests/ui/lang/core/array/init_array_i8.rs b/tests/compiletests/ui/lang/core/array/init_array_i8.rs index fda9b1abcd..f3e7a51ba5 100644 --- a/tests/compiletests/ui/lang/core/array/init_array_i8.rs +++ b/tests/compiletests/ui/lang/core/array/init_array_i8.rs @@ -1,5 +1,6 @@ // Test creating an array. // build-pass +// compile-flags: -C target-feature=+Int8 use spirv_std::macros::spirv; diff --git a/tests/compiletests/ui/lang/core/intrinsics/bswap.rs b/tests/compiletests/ui/lang/core/intrinsics/bswap.rs index 27589f1284..6aba9e52d1 100644 --- a/tests/compiletests/ui/lang/core/intrinsics/bswap.rs +++ b/tests/compiletests/ui/lang/core/intrinsics/bswap.rs @@ -1,5 +1,6 @@ // Test bswap intrinsic // build-pass +// compile-flags: -C target-feature=+Int8,+Int16,+Int64 #![allow(internal_features)] #![feature(core_intrinsics)] diff --git a/tests/compiletests/ui/lang/core/intrinsics/leading_zeros.rs b/tests/compiletests/ui/lang/core/intrinsics/leading_zeros.rs index d0009ffd1c..34f555fc9a 100644 --- a/tests/compiletests/ui/lang/core/intrinsics/leading_zeros.rs +++ b/tests/compiletests/ui/lang/core/intrinsics/leading_zeros.rs @@ -1,4 +1,5 @@ // build-pass +// compile-flags: -C target-feature=+Int8,+Int16,+Int64 use spirv_std::spirv; diff --git a/tests/compiletests/ui/lang/core/intrinsics/trailing_zeros.rs b/tests/compiletests/ui/lang/core/intrinsics/trailing_zeros.rs index a5de0cf946..ab460bb7ff 100644 --- a/tests/compiletests/ui/lang/core/intrinsics/trailing_zeros.rs +++ b/tests/compiletests/ui/lang/core/intrinsics/trailing_zeros.rs @@ -1,4 +1,5 @@ // build-pass +// compile-flags: -C target-feature=+Int8,+Int16,+Int64 use spirv_std::spirv; diff --git a/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.rs b/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.rs index 02596f69c3..4d49c6ce7a 100644 --- a/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.rs +++ b/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.rs @@ -9,9 +9,9 @@ use spirv_std::spirv; use core::ptr::Unique; -const POINTER: Unique<[u8; 4]> = Unique::<[u8; 4]>::dangling(); +const POINTER: Unique<()> = Unique::<()>::dangling(); #[spirv(fragment)] -pub fn main(output: &mut Unique<[u8; 4]>) { +pub fn main(output: &mut Unique<()>) { *output = POINTER; } diff --git a/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.stderr b/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.stderr index b52a31b6a3..f094f850fa 100644 --- a/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.stderr +++ b/tests/compiletests/ui/lang/core/ptr/allocate_const_scalar.stderr @@ -17,7 +17,7 @@ note: used from within `allocate_const_scalar::main` note: called by `main` --> $DIR/allocate_const_scalar.rs:15:8 | -15 | pub fn main(output: &mut Unique<[u8; 4]>) { +15 | pub fn main(output: &mut Unique<()>) { | ^^^^ error: aborting due to 1 previous error; 1 warning emitted diff --git a/tests/compiletests/ui/lang/u32/bit_reverse.rs b/tests/compiletests/ui/lang/u32/bit_reverse.rs index 41bfa8aac1..3214c82dfe 100644 --- a/tests/compiletests/ui/lang/u32/bit_reverse.rs +++ b/tests/compiletests/ui/lang/u32/bit_reverse.rs @@ -1,6 +1,7 @@ // Test all trailing and leading zeros. No need to test ones, they just call the zero variant with !value // build-pass +// compile-flags: -C target-feature=+Int8,+Int16,+Int64 use spirv_std::spirv; diff --git a/tests/compiletests/ui/lang/u32/count_ones.rs b/tests/compiletests/ui/lang/u32/count_ones.rs index 11058eea4c..30581cf7ff 100644 --- a/tests/compiletests/ui/lang/u32/count_ones.rs +++ b/tests/compiletests/ui/lang/u32/count_ones.rs @@ -1,6 +1,7 @@ // Test all trailing and leading zeros. No need to test ones, they just call the zero variant with !value // build-pass +// compile-flags: -C target-feature=+Int8,+Int16,+Int64 use spirv_std::spirv; From bff169a0a9031b7b6e4608ffb038ff08c9a80fac Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 10 Jul 2025 23:42:16 +0300 Subject: [PATCH 2/4] Handle unsupported int/float widths (`i128`/`u128`/`f128`) in `check_type_capabilities`. --- .../src/linker/simple_passes.rs | 91 +++++++++++-------- crates/rustc_codegen_spirv/src/spirv_type.rs | 27 +----- 2 files changed, 54 insertions(+), 64 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/simple_passes.rs b/crates/rustc_codegen_spirv/src/linker/simple_passes.rs index 32a47d12a3..e6f68ba8d1 100644 --- a/crates/rustc_codegen_spirv/src/linker/simple_passes.rs +++ b/crates/rustc_codegen_spirv/src/linker/simple_passes.rs @@ -1,4 +1,4 @@ -use super::{Result, get_name, get_names}; +use super::{get_name, get_names}; use rspirv::dr::{Block, Function, Module}; use rspirv::spirv::{Decoration, ExecutionModel, Op, Word}; use rustc_codegen_spirv_types::Capability; @@ -7,23 +7,32 @@ use rustc_session::Session; use std::iter::once; use std::mem::take; +/// Error marker type, indicating an integer/float type SPIR-V lacks support for. +struct UnsupportedType; + /// Returns the capability required for an integer type of the given width, if any. -fn capability_for_int_width(width: u32) -> Option { - match width { +fn capability_for_int_width( + width: u32, +) -> Result, UnsupportedType> { + Ok(match width { 8 => Some(rspirv::spirv::Capability::Int8), 16 => Some(rspirv::spirv::Capability::Int16), + 32 => None, 64 => Some(rspirv::spirv::Capability::Int64), - _ => None, - } + _ => return Err(UnsupportedType), + }) } /// Returns the capability required for a float type of the given width, if any. -fn capability_for_float_width(width: u32) -> Option { - match width { +fn capability_for_float_width( + width: u32, +) -> Result, UnsupportedType> { + Ok(match width { 16 => Some(rspirv::spirv::Capability::Float16), + 32 => None, 64 => Some(rspirv::spirv::Capability::Float64), - _ => None, - } + _ => return Err(UnsupportedType), + }) } pub fn shift_ids(module: &mut Module, add: u32) { @@ -177,7 +186,7 @@ pub fn name_variables_pass(module: &mut Module) { } // Some instructions are only valid in fragment shaders. Check them. -pub fn check_fragment_insts(sess: &Session, module: &Module) -> Result<()> { +pub fn check_fragment_insts(sess: &Session, module: &Module) -> super::Result<()> { let mut visited = vec![false; module.functions.len()]; let mut stack = Vec::new(); let mut names = None; @@ -219,7 +228,7 @@ pub fn check_fragment_insts(sess: &Session, module: &Module) -> Result<()> { names: &mut Option>, index: usize, func_id_to_idx: &FxHashMap, - ) -> Result<()> { + ) -> super::Result<()> { if visited[index] { return Ok(()); } @@ -288,7 +297,7 @@ pub fn check_fragment_insts(sess: &Session, module: &Module) -> Result<()> { /// /// This function validates that if a module uses types like u8/i8 (requiring Int8), /// u16/i16 (requiring Int16), etc., the corresponding capabilities are declared. -pub fn check_type_capabilities(sess: &Session, module: &Module) -> Result<()> { +pub fn check_type_capabilities(sess: &Session, module: &Module) -> super::Result<()> { use rspirv::spirv::Capability; // Collect declared capabilities @@ -298,44 +307,48 @@ pub fn check_type_capabilities(sess: &Session, module: &Module) -> Result<()> { .map(|inst| inst.operands[0].unwrap_capability()) .collect(); - let mut errors = Vec::new(); + let mut missing_caps = vec![]; for inst in &module.types_global_values { - match inst.class.opcode { + let (prefix, width, maybe_required_cap) = match inst.class.opcode { Op::TypeInt => { let width = inst.operands[0].unwrap_literal_bit32(); - let signedness = inst.operands[1].unwrap_literal_bit32() != 0; - let type_name = if signedness { "i" } else { "u" }; - - if let Some(required_cap) = capability_for_int_width(width) - && !declared_capabilities.contains(&required_cap) - { - errors.push(format!( - "`{type_name}{width}` type used without `OpCapability {required_cap:?}`" - )); - } + let signed = inst.operands[1].unwrap_literal_bit32() != 0; + + ( + if signed { "i" } else { "u" }, + width, + capability_for_int_width(width), + ) } Op::TypeFloat => { let width = inst.operands[0].unwrap_literal_bit32(); - if let Some(required_cap) = capability_for_float_width(width) - && !declared_capabilities.contains(&required_cap) - { - errors.push(format!( - "`f{width}` type used without `OpCapability {required_cap:?}`" - )); - } + ("f", width, capability_for_float_width(width)) } - _ => {} + _ => continue, + }; + + match maybe_required_cap { + Err(UnsupportedType) => { + sess.dcx() + .err(format!("`{prefix}{width}` unsupported in SPIR-V")); + } + Ok(Some(required_cap)) if !declared_capabilities.contains(&required_cap) => { + missing_caps.push(format!( + "`{prefix}{width}` type used without `OpCapability {required_cap:?}`" + )); + } + Ok(_) => {} } } - if !errors.is_empty() { + if !missing_caps.is_empty() { let mut err = sess .dcx() - .struct_err("Missing required capabilities for types"); - for error in errors { - err = err.with_note(error); + .struct_err("missing required capabilities for types"); + for msg in missing_caps { + err.note(msg); } Err(err.emit()) } else { @@ -359,13 +372,13 @@ pub fn remove_unused_type_capabilities(module: &mut Module) { match inst.class.opcode { Op::TypeInt => { let width = inst.operands[0].unwrap_literal_bit32(); - if let Some(cap) = capability_for_int_width(width) { + if let Ok(Some(cap)) = capability_for_int_width(width) { needed_type_capabilities.insert(cap); } } Op::TypeFloat => { let width = inst.operands[0].unwrap_literal_bit32(); - if let Some(cap) = capability_for_float_width(width) { + if let Ok(Some(cap)) = capability_for_float_width(width) { needed_type_capabilities.insert(cap); } } @@ -391,7 +404,7 @@ pub fn remove_unused_type_capabilities(module: &mut Module) { /// Remove all [`Decoration::NonUniform`] if this module does *not* have [`Capability::ShaderNonUniform`]. /// This allows image asm to always declare `NonUniform` and not worry about conditional compilation. -pub fn remove_non_uniform_decorations(_sess: &Session, module: &mut Module) -> Result<()> { +pub fn remove_non_uniform_decorations(_sess: &Session, module: &mut Module) -> super::Result<()> { let has_shader_non_uniform_capability = module.capabilities.iter().any(|inst| { inst.class.opcode == Op::Capability && inst.operands[0].unwrap_capability() == Capability::ShaderNonUniform diff --git a/crates/rustc_codegen_spirv/src/spirv_type.rs b/crates/rustc_codegen_spirv/src/spirv_type.rs index 18c5d59e34..074e5708d1 100644 --- a/crates/rustc_codegen_spirv/src/spirv_type.rs +++ b/crates/rustc_codegen_spirv/src/spirv_type.rs @@ -101,31 +101,8 @@ impl SpirvType<'_> { let result = match self { Self::Void => cx.emit_global().type_void_id(id), Self::Bool => cx.emit_global().type_bool_id(id), - Self::Integer(width, signedness) => { - let result = cx.emit_global().type_int_id(id, width, signedness as u32); - let u_or_i = if signedness { "i" } else { "u" }; - match width { - 8 | 16 | 32 | 64 => {} - w => cx.zombie_with_span( - result, - def_span, - &format!("`{u_or_i}{w}` unsupported in SPIR-V"), - ), - }; - result - } - Self::Float(width) => { - let result = cx.emit_global().type_float_id(id, width); - match width { - 16 | 32 | 64 => (), - other => cx.zombie_with_span( - result, - def_span, - &format!("`f{other}` unsupported in SPIR-V"), - ), - }; - result - } + Self::Integer(width, signed) => cx.emit_global().type_int_id(id, width, signed as u32), + Self::Float(width) => cx.emit_global().type_float_id(id, width), Self::Adt { def_id: _, align: _, From bdafea20f5e1d12058df52b07dfd5ee202c35667 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 17 Aug 2023 22:55:58 +0300 Subject: [PATCH 3/4] tests: add `dis/scalars.rs` test for `iN`/`uN`/`fN` types. --- tests/compiletests/ui/dis/asm_op_decorate.rs | 2 +- .../ui/dis/non-writable-storage_buffer.rs | 2 +- .../compiletests/ui/dis/scalars.nocaps.stderr | 16 ++++ tests/compiletests/ui/dis/scalars.rs | 74 +++++++++++++++ .../ui/dis/scalars.supported.stderr | 90 +++++++++++++++++++ .../ui/dis/target_features.stderr | 8 -- 6 files changed, 182 insertions(+), 10 deletions(-) create mode 100644 tests/compiletests/ui/dis/scalars.nocaps.stderr create mode 100644 tests/compiletests/ui/dis/scalars.rs create mode 100644 tests/compiletests/ui/dis/scalars.supported.stderr delete mode 100644 tests/compiletests/ui/dis/target_features.stderr diff --git a/tests/compiletests/ui/dis/asm_op_decorate.rs b/tests/compiletests/ui/dis/asm_op_decorate.rs index a9b227e012..604e1a241c 100644 --- a/tests/compiletests/ui/dis/asm_op_decorate.rs +++ b/tests/compiletests/ui/dis/asm_op_decorate.rs @@ -12,7 +12,7 @@ // normalize-stderr-test "ui/dis/" -> "$$DIR/" // FIXME(eddyb) this should use revisions to track both the `vulkan1.2` output -// and the pre-`vulkan1.2` output, but per-revisions `{only,ignore}-*` directives +// and the pre-`vulkan1.2` output, but per-revision `{only,ignore}-*` directives // are not supported in `compiletest-rs`. // ignore-vulkan1.2 // ignore-vulkan1.3 diff --git a/tests/compiletests/ui/dis/non-writable-storage_buffer.rs b/tests/compiletests/ui/dis/non-writable-storage_buffer.rs index 3b53078ce2..836eed62b9 100644 --- a/tests/compiletests/ui/dis/non-writable-storage_buffer.rs +++ b/tests/compiletests/ui/dis/non-writable-storage_buffer.rs @@ -13,7 +13,7 @@ // normalize-stderr-test "ui/dis/" -> "$$DIR/" // FIXME(eddyb) this should use revisions to track both the `vulkan1.2` output -// and the pre-`vulkan1.2` output, but per-revisions `{only,ignore}-*` directives +// and the pre-`vulkan1.2` output, but per-revision `{only,ignore}-*` directives // are not supported in `compiletest-rs`. // ignore-vulkan1.2 // ignore-vulkan1.3 diff --git a/tests/compiletests/ui/dis/scalars.nocaps.stderr b/tests/compiletests/ui/dis/scalars.nocaps.stderr new file mode 100644 index 0000000000..6e38c343a5 --- /dev/null +++ b/tests/compiletests/ui/dis/scalars.nocaps.stderr @@ -0,0 +1,16 @@ +error: `u128` unsupported in SPIR-V + +error: `i128` unsupported in SPIR-V + +error: missing required capabilities for types + | + = note: `u8` type used without `OpCapability Int8` + = note: `u16` type used without `OpCapability Int16` + = note: `u64` type used without `OpCapability Int64` + = note: `i8` type used without `OpCapability Int8` + = note: `i16` type used without `OpCapability Int16` + = note: `i64` type used without `OpCapability Int64` + = note: `f64` type used without `OpCapability Float64` + +error: aborting due to 3 previous errors + diff --git a/tests/compiletests/ui/dis/scalars.rs b/tests/compiletests/ui/dis/scalars.rs new file mode 100644 index 0000000000..b8a87616e6 --- /dev/null +++ b/tests/compiletests/ui/dis/scalars.rs @@ -0,0 +1,74 @@ +#![crate_name = "scalars"] + +// Tests all the (supported) Rust integer/floating-point scalar types. + +// revisions: supported nocaps +//[supported] build-pass +//[supported] compile-flags: -C target-feature=+Int8,+Int16,+Int64,+Float64 +//[nocaps] build-fail + +// compile-flags: -C llvm-args=--disassemble-globals +// normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> "" +// normalize-stderr-test "OpSource .*\n" -> "" +// normalize-stderr-test "OpExtension .SPV_KHR_vulkan_memory_model.\n" -> "" +// normalize-stderr-test "OpMemoryModel Logical Vulkan" -> "OpMemoryModel Logical Simple" + +// HACK(eddyb) `compiletest` handles `ui\dis\`, but not `ui\\dis\\`, on Windows. +// normalize-stderr-test "ui/dis/" -> "$$DIR/" + +use spirv_std::spirv; + +#[spirv(fragment)] +pub fn main( + out: &mut u32, + + #[spirv(flat)] in_u8: u8, + #[spirv(flat)] in_u16: u16, + #[spirv(flat)] in_u32: u32, + #[spirv(flat)] in_u64: u64, + // FIXME(eddyb) support `u128` somehow! + #[cfg(not(supported))] + #[spirv(flat)] + in_u128: u128, + + #[spirv(flat)] in_i8: i8, + #[spirv(flat)] in_i16: i16, + #[spirv(flat)] in_i32: i32, + #[spirv(flat)] in_i64: i64, + // FIXME(eddyb) support `i128` somehow! + #[cfg(not(supported))] + #[spirv(flat)] + in_i128: i128, + + in_f32: f32, + #[spirv(flat)] in_f64: f64, +) { + // HACK(eddyb) to make it more obvious in the disassembly, each type gets a + // constant with its bit width, disambiguated for signed integers by negation. + *out |= (in_u8 * 8) as u32; + *out |= (in_u16 * 16) as u32; + *out |= (in_u32 * 32) as u32; + *out |= (in_u64 * 64) as u32; + // FIXME(eddyb) support `u128` somehow! + #[cfg(not(supported))] + { + // FIXME(eddyb) constants still emit zombies that get reported too early. + // *out |= (in_u128 * 128) as u32; + *out |= (in_u128 + in_u128) as u32; + } + + *out |= (in_i8 * -8) as u32; + *out |= (in_i16 * -16) as u32; + *out |= (in_i32 * -32) as u32; + *out |= (in_i64 * -64) as u32; + // FIXME(eddyb) support `i128` somehow! + #[cfg(not(supported))] + { + // FIXME(eddyb) constants still emit zombies that get reported too early. + // *out |= (in_i128 * -128) as u32; + *out |= (in_i128 + in_i128) as u32; + } + + *out |= (in_f32 * 32.0) as u32; + *out |= (in_f64 * 64.0) as u32; +} diff --git a/tests/compiletests/ui/dis/scalars.supported.stderr b/tests/compiletests/ui/dis/scalars.supported.stderr new file mode 100644 index 0000000000..595406ec94 --- /dev/null +++ b/tests/compiletests/ui/dis/scalars.supported.stderr @@ -0,0 +1,90 @@ +OpCapability Shader +OpCapability Float64 +OpCapability Int64 +OpCapability Int16 +OpCapability Int8 +OpMemoryModel Logical Simple +OpEntryPoint Fragment %1 "main" %2 %3 %4 %5 %6 %7 %8 %9 %10 %11 %12 +OpExecutionMode %1 OriginUpperLeft +%13 = OpString "$DIR/scalars.rs" +OpName %3 "in_u8" +OpName %4 "in_u16" +OpName %5 "in_u32" +OpName %6 "in_u64" +OpName %7 "in_i8" +OpName %8 "in_i16" +OpName %9 "in_i32" +OpName %10 "in_i64" +OpName %11 "in_f32" +OpName %12 "in_f64" +OpName %2 "out" +OpDecorate %3 Flat +OpDecorate %3 Location 0 +OpDecorate %4 Flat +OpDecorate %4 Location 1 +OpDecorate %5 Flat +OpDecorate %5 Location 2 +OpDecorate %6 Flat +OpDecorate %6 Location 3 +OpDecorate %7 Flat +OpDecorate %7 Location 4 +OpDecorate %8 Flat +OpDecorate %8 Location 5 +OpDecorate %9 Flat +OpDecorate %9 Location 6 +OpDecorate %10 Flat +OpDecorate %10 Location 7 +OpDecorate %11 Location 8 +OpDecorate %12 Flat +OpDecorate %12 Location 9 +OpDecorate %2 Location 0 +%14 = OpTypeInt 32 0 +%15 = OpTypePointer Output %14 +%16 = OpTypeInt 8 0 +%17 = OpTypePointer Input %16 +%18 = OpTypeInt 16 0 +%19 = OpTypePointer Input %18 +%20 = OpTypePointer Input %14 +%21 = OpTypeInt 64 0 +%22 = OpTypePointer Input %21 +%23 = OpTypeInt 8 1 +%24 = OpTypePointer Input %23 +%25 = OpTypeInt 16 1 +%26 = OpTypePointer Input %25 +%27 = OpTypeInt 32 1 +%28 = OpTypePointer Input %27 +%29 = OpTypeInt 64 1 +%30 = OpTypePointer Input %29 +%31 = OpTypeFloat 32 +%32 = OpTypePointer Input %31 +%33 = OpTypeFloat 64 +%34 = OpTypePointer Input %33 +%35 = OpTypeVoid +%36 = OpTypeFunction %35 +%3 = OpVariable %17 Input +%4 = OpVariable %19 Input +%5 = OpVariable %20 Input +%6 = OpVariable %22 Input +%7 = OpVariable %24 Input +%8 = OpVariable %26 Input +%9 = OpVariable %28 Input +%10 = OpVariable %30 Input +%11 = OpVariable %32 Input +%12 = OpVariable %34 Input +%37 = OpConstant %16 8 +%2 = OpVariable %15 Output +%38 = OpConstant %18 16 +%39 = OpConstant %14 32 +%40 = OpConstant %21 64 +%41 = OpConstant %23 4294967288 +%42 = OpConstant %25 4294967280 +%43 = OpConstant %27 4294967264 +%44 = OpConstant %29 18446744073709551552 +%45 = OpConstant %31 1107296256 +%46 = OpConstant %14 0 +%47 = OpConstant %14 1333788671 +%48 = OpTypeBool +%49 = OpConstant %14 4294967295 +%50 = OpConstant %33 4634204016564240384 +%51 = OpConstant %21 0 +%52 = OpConstant %21 4751297606873776128 diff --git a/tests/compiletests/ui/dis/target_features.stderr b/tests/compiletests/ui/dis/target_features.stderr deleted file mode 100644 index 52abfb69ef..0000000000 --- a/tests/compiletests/ui/dis/target_features.stderr +++ /dev/null @@ -1,8 +0,0 @@ -OpCapability RayTracingKHR -OpCapability Shader -OpExtension "SPV_KHR_ray_tracing" -OpMemoryModel Logical Simple -OpEntryPoint AnyHitNV %1 "main" -OpName %2 "target_features::main" -%3 = OpTypeVoid -%4 = OpTypeFunction %3 From 3709a5eb32e4d592e8f9bc3506970c4541a48d0d Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 25 Nov 2023 09:51:05 +0200 Subject: [PATCH 4/4] Minimal SPIR-T-based validation of extensions and capabilities (esp. `IntN`/`FloatN`). --- crates/rustc_codegen_spirv/src/linker/mod.rs | 23 +- .../src/linker/simple_passes.rs | 66 +-- .../src/linker/spirt_passes/mod.rs | 5 +- .../src/linker/spirt_passes/validate.rs | 414 ++++++++++++++++++ .../compiletests/ui/dis/scalars.nocaps.stderr | 86 +++- 5 files changed, 507 insertions(+), 87 deletions(-) create mode 100644 crates/rustc_codegen_spirv/src/linker/spirt_passes/validate.rs diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 41a46a03e4..d1e704cab7 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -481,16 +481,6 @@ pub fn link( simple_passes::remove_non_uniform_decorations(sess, &mut output)?; } - { - let _timer = sess.timer("link_remove_unused_type_capabilities"); - simple_passes::remove_unused_type_capabilities(&mut output); - } - - { - let _timer = sess.timer("link_type_capability_check"); - simple_passes::check_type_capabilities(sess, &output)?; - } - // NOTE(eddyb) SPIR-T pipeline is entirely limited to this block. { let (spv_words, module_or_err, lower_from_spv_timer) = @@ -563,10 +553,16 @@ pub fn link( module, &opts.spirt_passes, |name, _module| before_pass(name), - after_pass, + &mut after_pass, ); } + { + let timer = before_pass("spirt_passes::validate"); + spirt_passes::validate::validate(module); + after_pass("validate", module, timer); + } + { let _timer = before_pass("spirt_passes::diagnostics::report_diagnostics"); spirt_passes::diagnostics::report_diagnostics(sess, opts, module).map_err( @@ -634,6 +630,11 @@ pub fn link( } } + { + let _timer = sess.timer("link_remove_unused_type_capabilities"); + simple_passes::remove_unused_type_capabilities(&mut output); + } + { let _timer = sess.timer("link_gather_all_interface_vars_from_uses"); entry_interface::gather_all_interface_vars_from_uses(&mut output); diff --git a/crates/rustc_codegen_spirv/src/linker/simple_passes.rs b/crates/rustc_codegen_spirv/src/linker/simple_passes.rs index e6f68ba8d1..bd8f614569 100644 --- a/crates/rustc_codegen_spirv/src/linker/simple_passes.rs +++ b/crates/rustc_codegen_spirv/src/linker/simple_passes.rs @@ -293,74 +293,14 @@ pub fn check_fragment_insts(sess: &Session, module: &Module) -> super::Result<() } } -/// Check that types requiring specific capabilities have those capabilities declared. -/// -/// This function validates that if a module uses types like u8/i8 (requiring Int8), -/// u16/i16 (requiring Int16), etc., the corresponding capabilities are declared. -pub fn check_type_capabilities(sess: &Session, module: &Module) -> super::Result<()> { - use rspirv::spirv::Capability; - - // Collect declared capabilities - let declared_capabilities: FxHashSet = module - .capabilities - .iter() - .map(|inst| inst.operands[0].unwrap_capability()) - .collect(); - - let mut missing_caps = vec![]; - - for inst in &module.types_global_values { - let (prefix, width, maybe_required_cap) = match inst.class.opcode { - Op::TypeInt => { - let width = inst.operands[0].unwrap_literal_bit32(); - let signed = inst.operands[1].unwrap_literal_bit32() != 0; - - ( - if signed { "i" } else { "u" }, - width, - capability_for_int_width(width), - ) - } - Op::TypeFloat => { - let width = inst.operands[0].unwrap_literal_bit32(); - - ("f", width, capability_for_float_width(width)) - } - _ => continue, - }; - - match maybe_required_cap { - Err(UnsupportedType) => { - sess.dcx() - .err(format!("`{prefix}{width}` unsupported in SPIR-V")); - } - Ok(Some(required_cap)) if !declared_capabilities.contains(&required_cap) => { - missing_caps.push(format!( - "`{prefix}{width}` type used without `OpCapability {required_cap:?}`" - )); - } - Ok(_) => {} - } - } - - if !missing_caps.is_empty() { - let mut err = sess - .dcx() - .struct_err("missing required capabilities for types"); - for msg in missing_caps { - err.note(msg); - } - Err(err.emit()) - } else { - Ok(()) - } -} - /// Remove type-related capabilities that are not required by any types in the module. /// /// This function specifically targets Int8, Int16, Int64, Float16, and Float64 capabilities, /// removing them if no types in the module require them. All other capabilities are preserved. /// This is part of the fix for issue #300 where constant casts were creating unnecessary types. +// +// FIXME(eddyb) move this to a SPIR-T pass (potentially even using sets of used +// exts/caps that validation itself can collect while traversing the module). pub fn remove_unused_type_capabilities(module: &mut Module) { use rspirv::spirv::Capability; diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs index 96f878518b..ce52895cba 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs @@ -5,6 +5,7 @@ pub(crate) mod debuginfo; pub(crate) mod diagnostics; mod fuse_selects; mod reduce; +pub(crate) mod validate; use lazy_static::lazy_static; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; @@ -63,7 +64,7 @@ macro_rules! def_spv_spec_with_extra_well_known { let spv_spec = spv::spec::Spec::get(); let wk = &spv_spec.well_known; - let decorations = match &spv_spec.operand_kinds[wk.Decoration] { + let decorations = match wk.Decoration.def() { spv::spec::OperandKindDef::ValueEnum { variants } => variants, _ => unreachable!(), }; @@ -101,7 +102,9 @@ def_spv_spec_with_extra_well_known! { OpCompositeExtract, ], operand_kind: spv::spec::OperandKind = [ + Capability, ExecutionModel, + ImageFormat, ], decoration: u32 = [ UserTypeGOOGLE, diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/validate.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/validate.rs new file mode 100644 index 0000000000..b528d5e9f1 --- /dev/null +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/validate.rs @@ -0,0 +1,414 @@ +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; +use spirt::func_at::FuncAtMut; +use spirt::transform::{InnerInPlaceTransform, InnerTransform, Transformed, Transformer}; +use spirt::{ + Const, ConstDef, ConstKind, Context, DataInst, DataInstForm, DataInstKind, Diag, Func, + GlobalVar, Module, ModuleDialect, Type, TypeDef, TypeKind, spv, +}; +use std::collections::VecDeque; + +pub fn validate(module: &mut Module) { + let spv_spec = super::SpvSpecWithExtras::get(); + let wk = &spv_spec.well_known; + + let mut validator = Validator { + cx: &module.cx(), + wk, + spv_spec_caps: match wk.Capability.def() { + spv::spec::OperandKindDef::ValueEnum { variants } => variants, + _ => unreachable!(), + }, + + module_spv_dialect: match &module.dialect { + ModuleDialect::Spv(dialect) => dialect, + }, + + transformed_types: FxHashMap::default(), + transformed_consts: FxHashMap::default(), + transformed_data_inst_forms: FxHashMap::default(), + seen_global_vars: FxHashSet::default(), + global_var_queue: VecDeque::new(), + seen_funcs: FxHashSet::default(), + func_queue: VecDeque::new(), + }; + + // Seed the queues starting from the module exports. + for exportee in module.exports.values_mut() { + exportee + .inner_transform_with(&mut validator) + .apply_to(exportee); + } + + // Process the queues until they're all empty. + while !validator.global_var_queue.is_empty() || !validator.func_queue.is_empty() { + while let Some(gv) = validator.global_var_queue.pop_front() { + validator.in_place_transform_global_var_decl(&mut module.global_vars[gv]); + } + while let Some(func) = validator.func_queue.pop_front() { + validator.in_place_transform_func_decl(&mut module.funcs[func]); + } + } +} + +struct Validator<'a> { + cx: &'a Context, + wk: &'static super::SpvWellKnownWithExtras, + spv_spec_caps: &'static spv::spec::indexed::NamedIdxMap< + u16, + spv::spec::Enumerant, + spv::spec::indexed::KhrSegmented, + >, + + module_spv_dialect: &'a spv::Dialect, + + // FIXME(eddyb) build some automation to avoid ever repeating these. + transformed_types: FxHashMap>, + transformed_consts: FxHashMap>, + transformed_data_inst_forms: FxHashMap>, + seen_global_vars: FxHashSet, + global_var_queue: VecDeque, + seen_funcs: FxHashSet, + func_queue: VecDeque, +} + +impl Transformer for Validator<'_> { + // FIXME(eddyb) build some automation to avoid ever repeating these. + fn transform_type_use(&mut self, ty: Type) -> Transformed { + if let Some(&cached) = self.transformed_types.get(&ty) { + return cached; + } + let transformed = self + .transform_type_def(&self.cx[ty]) + .map(|ty_def| self.cx.intern(ty_def)); + self.transformed_types.insert(ty, transformed); + transformed + } + fn transform_const_use(&mut self, ct: Const) -> Transformed { + if let Some(&cached) = self.transformed_consts.get(&ct) { + return cached; + } + let transformed = self + .transform_const_def(&self.cx[ct]) + .map(|ct_def| self.cx.intern(ct_def)); + self.transformed_consts.insert(ct, transformed); + transformed + } + fn transform_data_inst_form_use( + &mut self, + data_inst_form: DataInstForm, + ) -> Transformed { + if let Some(&cached) = self.transformed_data_inst_forms.get(&data_inst_form) { + return cached; + } + let transformed = self + .transform_data_inst_form_def(&self.cx[data_inst_form]) + .map(|data_inst_form_def| self.cx.intern(data_inst_form_def)); + self.transformed_data_inst_forms + .insert(data_inst_form, transformed); + transformed + } + + fn transform_global_var_use(&mut self, gv: GlobalVar) -> Transformed { + if self.seen_global_vars.insert(gv) { + self.global_var_queue.push_back(gv); + } + Transformed::Unchanged + } + fn transform_func_use(&mut self, func: Func) -> Transformed { + if self.seen_funcs.insert(func) { + self.func_queue.push_back(func); + } + Transformed::Unchanged + } + + // NOTE(eddyb) above methods are plumbing, validation methods are below. + + fn transform_type_def(&mut self, ty_def: &TypeDef) -> Transformed { + let valid = match &ty_def.kind { + TypeKind::SpvInst { + spv_inst, + type_and_const_inputs: _, + } => self.validate_spv_inst(spv_inst), + + TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => Ok(()), + }; + let transformed = ty_def.inner_transform_with(self); + match valid { + Ok(()) => transformed, + Err(diag) => { + let mut ty_def = TypeDef { + attrs: ty_def.attrs, + kind: ty_def.kind.clone(), + }; + transformed.apply_to(&mut ty_def); + ty_def.attrs.push_diag(self.cx, diag); + Transformed::Changed(ty_def) + } + } + } + + fn transform_const_def(&mut self, ct_def: &ConstDef) -> Transformed { + let valid = match &ct_def.kind { + ConstKind::SpvInst { + spv_inst_and_const_inputs, + } => { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; + self.validate_spv_inst(spv_inst) + } + + ConstKind::PtrToGlobalVar(_) | ConstKind::SpvStringLiteralForExtInst(_) => Ok(()), + }; + let transformed = ct_def.inner_transform_with(self); + match valid { + Ok(()) => transformed, + Err(diag) => { + let mut ct_def = ConstDef { + attrs: ct_def.attrs, + ty: ct_def.ty, + kind: ct_def.kind.clone(), + }; + transformed.apply_to(&mut ct_def); + ct_def.attrs.push_diag(self.cx, diag); + Transformed::Changed(ct_def) + } + } + } + + fn in_place_transform_data_inst_def(&mut self, mut func_at_data_inst: FuncAtMut<'_, DataInst>) { + func_at_data_inst + .reborrow() + .inner_in_place_transform_with(self); + + let inst_def = func_at_data_inst.def(); + let valid = match &self.cx[inst_def.form].kind { + DataInstKind::SpvInst(spv_inst) => self.validate_spv_inst(spv_inst), + + DataInstKind::FuncCall(_) | DataInstKind::QPtr(_) | DataInstKind::SpvExtInst { .. } => { + Ok(()) + } + }; + if let Err(diag) = valid { + inst_def.attrs.push_diag(self.cx, diag); + } + } +} + +impl Validator<'_> { + fn require_spv_exts_caps<'a>( + &self, + describe: impl FnOnce() -> String, + exts_providing: impl ExactSizeIterator + Clone, + caps_enabling: impl ExactSizeIterator + Clone, + ) -> Result<(), Diag> { + // FIXME(eddyb) find a consistent style between all the error messages. + let provided_by_core_spv_or_ext = exts_providing.len() == 0 + || exts_providing.clone().any(|ext| { + self.module_spv_dialect.extensions.contains(ext) + || min_spv_version_implying_ext(ext).is_some_and(|min_version| { + let module_version = { + let d = &self.module_spv_dialect; + (d.version_major, d.version_minor) + }; + module_version >= min_version + }) + }); + if !provided_by_core_spv_or_ext { + let exts = exts_providing + .map(|ext| format!("`{ext}`")) + .collect::>() + .join(", "); + return Err(Diag::err([ + describe().into(), + format!(" requires one of these extensions: {exts}",).into(), + ])); + } + + let enabled_by_default_or_cap = caps_enabling.len() == 0 + || caps_enabling + .clone() + .any(|cap| self.module_spv_dialect.capabilities.contains(&cap)) + || { + // HACK(eddyb) this is an expensive fallback, and should be + // precomputed ahead of time, but `spirt::spv::spec` exposing + // the necessary information is the bigger issue and should be + // solved first, before optimizing any of this. + let mut elabored_module_caps: FxIndexSet<_> = self + .module_spv_dialect + .capabilities + .iter() + .copied() + .collect(); + let mut i = 0; + while i < elabored_module_caps.len() { + let cap = elabored_module_caps[i]; + if let Some(cap) = rspirv::spirv::Capability::from_u32(cap) { + elabored_module_caps.extend( + rspirv::dr::Operand::from(cap) + .required_capabilities() + .into_iter() + .map(|cap| cap as u32), + ); + } + i += 1; + } + caps_enabling + .clone() + .any(|cap| elabored_module_caps.contains(&cap)) + }; + if !enabled_by_default_or_cap { + let caps = caps_enabling + .map(|cap| { + let cap_name = u16::try_from(cap) + .ok() + .and_then(|cap| Some(self.spv_spec_caps.get_named(cap)?.0)); + match cap_name { + Some(name) => format!("`{name}`"), + None => format!(""), + } + }) + .collect::>() + .join(", "); + return Err(Diag::err([ + describe().into(), + format!(" requires one of these capabilities: {caps}",).into(), + ])); + } + Ok(()) + } + + fn validate_spv_inst(&self, spv_inst: &spv::Inst) -> Result<(), Diag> { + // FIXME(eddyb) make this information available through `spirt::spv::spec`. + let (exts_providing_inst, caps_enabling_insts) = { + let inst_def = + rspirv::grammar::CoreInstructionTable::lookup_opcode(spv_inst.opcode.as_u16()) + .unwrap(); + ( + inst_def.extensions.iter().copied(), + inst_def.capabilities.iter().map(|&cap| cap as u32), + ) + }; + self.require_spv_exts_caps( + || format!("SPIR-V `{}` instruction", spv_inst.opcode.name()), + exts_providing_inst, + caps_enabling_insts, + )?; + + // HACK(eddyb) even if this seems wasteful in its allocation of + // strings, they should only happen once each per module, and + // also it wouldn't be hard to switch to some "small str" crate. + let int_or_float = |type_name: &str, cap_name: &str| { + // FIXME(eddyb) find a consistent style between all the error messages + // (mentioning `OpCapability` seems unfortunate, for example). + match self.spv_spec_caps.lookup(cap_name).map(u32::from) { + None => Err(format!("`{type_name}` type unsupported in SPIR-V")), + Some(cap) if !self.module_spv_dialect.capabilities.contains(&cap) => Err(format!( + "`{type_name}` type used without `OpCapability {cap_name}`" + )), + Some(_) => Ok(()), + } + .map_err(|msg| Diag::err([msg.into()])) + }; + match spv_inst.imms[..] { + [spv::Imm::Short(_, width), spv::Imm::Short(_, signedness)] + if spv_inst.opcode == self.wk.OpTypeInt && width != 32 => + { + let signed = signedness != 0; + int_or_float( + &format!("{}{width}", if signed { "i" } else { "u" }), + &format!("Int{width}"), + )?; + } + [spv::Imm::Short(_, width)] + if spv_inst.opcode == self.wk.OpTypeFloat && width != 32 => + { + int_or_float(&format!("f{width}"), &format!("Float{width}"))?; + } + _ => {} + } + + // FIXME(eddyb) implement this after exposing enough of the information + // via `spirt::spv::spec` (will likely need some way to efficiently store + // `(extensions, capabilities)`, e.g. ad-hoc interning into one integer, + // because that will have to be added to every single e.g. `Enumerant`). + for &imm in &spv_inst.imms { + // HACK(eddyb) two simple cases being handled via `rspirv` as a demo. + let check_enum_via_rspirv = |enum_kind: spv::spec::OperandKind, + // HACK(eddyb) bypassing rustfmt failure mode. + #[allow(unused_parens)] as_rspirv_operand: ( + fn(u32) -> Option + )| { + if let spv::Imm::Short(kind, imm) = imm + && kind == enum_kind + && let Some(operand) = as_rspirv_operand(imm) + { + self.require_spv_exts_caps( + || { + let (enum_name, enum_def) = kind.name_and_def(); + let enumerant_name = match enum_def { + spv::spec::OperandKindDef::ValueEnum { variants } => { + u16::try_from(imm) + .ok() + .and_then(|imm| Some(variants.get_named(imm)?.0)) + } + _ => None, + }; + match enumerant_name { + Some(name) => format!("SPIR-V `{enum_name}.{name}` operand"), + None => format!(""), + } + }, + operand.required_extensions().iter().copied(), + operand + .required_capabilities() + .iter() + .map(|&cap| cap as u32), + )?; + } + + Ok(()) + }; + check_enum_via_rspirv(self.wk.ImageFormat, |imm| { + Some(rspirv::spirv::ImageFormat::from_u32(imm)?.into()) + })?; + check_enum_via_rspirv(self.wk.StorageClass, |imm| { + Some(rspirv::spirv::StorageClass::from_u32(imm)?.into()) + })?; + } + + Ok(()) + } +} + +// HACK(eddyb) due to neither SPIR-T, nor `rspirv`, exposing the SPIR-V version +// which started provided an instruction/enumerator/etc. (i.e. supplanting the +// need for an extension), this "first SPIR-V version incorporating an extension" +// approximation was obtained by running (inside `spirt 0.4.0`'s source tree): +// +// jq -r '[(.instructions[], (.operand_kinds[] | (.enumerants//[])[])) \ +// | {e:.extensions|arrays|select(length>0)|unique,v:.version}] as $all \ +// | [$all[].e[]] | unique | map(\ +// . as $e | $all | map(select(.e|contains([$e])) | (.v//"None")) \ +// | max | select(. != "None") | {e:$e,v:(split(".") | map(tonumber))}\ +// ) \ +// | group_by(.v) | map({e:map(@json"\(.e)")|join("\n| "), v:.[0].v})[] \ +// | "\(.e) => (\(.v[0]), \(.v[1])),"' \ +// khronos-spec/SPIRV-Headers/include/spirv/unified1/spirv.core.grammar.json +// +fn min_spv_version_implying_ext(ext: &str) -> Option<(u8, u8)> { + Some(match ext { + "SPV_KHR_16bit_storage" + | "SPV_KHR_device_group" + | "SPV_KHR_multiview" + | "SPV_KHR_shader_draw_parameters" + | "SPV_KHR_storage_buffer_storage_class" + | "SPV_KHR_variable_pointers" => (1, 3), + "SPV_GOOGLE_decorate_string" | "SPV_KHR_no_integer_wrap_decoration" => (1, 4), + "SPV_EXT_descriptor_indexing" + | "SPV_EXT_physical_storage_buffer" + | "SPV_KHR_8bit_storage" + | "SPV_KHR_physical_storage_buffer" + | "SPV_KHR_vulkan_memory_model" => (1, 5), + "SPV_KHR_integer_dot_product" | "SPV_KHR_terminate_invocation" => (1, 6), + _ => return None, + }) +} diff --git a/tests/compiletests/ui/dis/scalars.nocaps.stderr b/tests/compiletests/ui/dis/scalars.nocaps.stderr index 6e38c343a5..f8bcf199d7 100644 --- a/tests/compiletests/ui/dis/scalars.nocaps.stderr +++ b/tests/compiletests/ui/dis/scalars.nocaps.stderr @@ -1,16 +1,78 @@ -error: `u128` unsupported in SPIR-V +error: `u8` type used without `OpCapability Int8` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:25:5 + | +25 | #[spirv(flat)] in_u8: u8, + | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: `i128` unsupported in SPIR-V +error: `u16` type used without `OpCapability Int16` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:26:5 + | +26 | #[spirv(flat)] in_u16: u16, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: missing required capabilities for types - | - = note: `u8` type used without `OpCapability Int8` - = note: `u16` type used without `OpCapability Int16` - = note: `u64` type used without `OpCapability Int64` - = note: `i8` type used without `OpCapability Int8` - = note: `i16` type used without `OpCapability Int16` - = note: `i64` type used without `OpCapability Int64` - = note: `f64` type used without `OpCapability Float64` +error: `u64` type used without `OpCapability Int64` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:28:5 + | +28 | #[spirv(flat)] in_u64: u64, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: `u128` type unsupported in SPIR-V + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:30:5 + | +30 | / #[cfg(not(supported))] +31 | | #[spirv(flat)] +32 | | in_u128: u128, + | |_________________^ + +error: `i8` type used without `OpCapability Int8` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:34:5 + | +34 | #[spirv(flat)] in_i8: i8, + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `i16` type used without `OpCapability Int16` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:35:5 + | +35 | #[spirv(flat)] in_i16: i16, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `i64` type used without `OpCapability Int64` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:37:5 + | +37 | #[spirv(flat)] in_i64: i64, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `i128` type unsupported in SPIR-V + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:39:5 + | +39 | / #[cfg(not(supported))] +40 | | #[spirv(flat)] +41 | | in_i128: i128, + | |_________________^ + +error: `f64` type used without `OpCapability Float64` + | +note: used from within Fragment entry-point `main` + --> $DIR/scalars.rs:44:5 + | +44 | #[spirv(flat)] in_f64: f64, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors