From 3a53968817a2fc061f16cb3e08899d49d25e74b2 Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Mon, 29 Mar 2021 18:43:07 +0200 Subject: [PATCH] Add support for multiple target environments (#533) * Add support for multiple target environments * updates from code review * Update lib.rs --- .github/workflows/test.sh | 2 +- crates/rustc_codegen_spirv/src/lib.rs | 40 +++-- crates/rustc_codegen_spirv/src/link.rs | 2 +- crates/spirv-builder/src/test/basic.rs | 25 +-- crates/spirv-builder/src/test/mod.rs | 13 -- docs/src/testing.md | 11 ++ tests/src/main.rs | 203 ++++++++++++++---------- tests/ui/image/fetch.rs | 5 +- tests/ui/image/issue_527.rs | 25 +++ tests/ui/image/read.rs | 6 +- tests/ui/image/sample.rs | 8 +- tests/ui/image/sample_gradient.rs | 8 +- tests/ui/image/sample_lod.rs | 8 +- tests/ui/image/write.rs | 6 +- tests/ui/storage_class/push_constant.rs | 6 +- 15 files changed, 203 insertions(+), 165 deletions(-) create mode 100644 tests/ui/image/issue_527.rs diff --git a/.github/workflows/test.sh b/.github/workflows/test.sh index 8e904efc63..668ff88be2 100755 --- a/.github/workflows/test.sh +++ b/.github/workflows/test.sh @@ -52,4 +52,4 @@ cargo_test_no_features examples/runners/cpu cargo_test_no_features examples/shaders/sky-shader cargo_test_no_features examples/shaders/simplest-shader -cargo compiletest +cargo compiletest --target-env unknown,vulkan1.1,spv1.3 diff --git a/crates/rustc_codegen_spirv/src/lib.rs b/crates/rustc_codegen_spirv/src/lib.rs index 2e71725a55..b6912da4a7 100644 --- a/crates/rustc_codegen_spirv/src/lib.rs +++ b/crates/rustc_codegen_spirv/src/lib.rs @@ -211,7 +211,7 @@ fn is_blocklisted_fn<'tcx>( false } -fn target_options() -> Target { +fn target_options(env: Option) -> Target { Target { llvm_target: "no-llvm".to_string(), pointer_width: 32, @@ -228,6 +228,10 @@ fn target_options() -> Target { linker_flavor: LinkerFlavor::Ld, panic_strategy: PanicStrategy::Abort, os: "unknown".to_string(), + env: env + .as_ref() + .map(ToString::to_string) + .unwrap_or_else(|| "unknown".to_string()), // TODO: Investigate if main_needs_argc_argv is useful (for building exes) main_needs_argc_argv: false, ..Default::default() @@ -290,12 +294,20 @@ impl CodegenBackend for SpirvCodegenBackend { fn target_override(&self, opts: &config::Options) -> Option { match opts.target_triple { - TargetTriple::TargetTriple(ref target_triple) => match &**target_triple { - // TODO: Do we want to match *everything* here, since, well, we only support one thing? that will allow - // folks to not specify --target spirv-unknown-unknown on the commandline. - "spirv-unknown-unknown" => Some(target_options()), - _ => None, - }, + TargetTriple::TargetTriple(ref target_triple) => { + const ARCH_VENDOR: &str = "spirv-unknown-"; + if !target_triple.starts_with(ARCH_VENDOR) { + return None; + } + + let env = &target_triple[ARCH_VENDOR.len()..]; + + match env.parse() { + Ok(env) => Some(target_options(Some(env))), + Err(_) if env == "unknown" => Some(target_options(None)), + Err(_) => None, + } + } TargetTriple::TargetPath(_) => None, } } @@ -640,17 +652,3 @@ pub fn __rustc_codegen_backend() -> Box { Box::new(SpirvCodegenBackend) } - -// HACK(eddyb) this allows `spirv-builder` to use `spirv-tools::val` without -// risking linker errors (especially when compiled with optimizations) - this -// also means the function can't be generic or `#[inline]`. -pub use spirv_tools::TargetEnv as SpirvToolsTargetEnv; -#[inline(never)] -pub fn spirv_tools_validate( - target_env: Option, - bytes: &[u8], - options: Option, -) -> Result<(), spirv_tools::error::Error> { - use spirv_tools::val::Validator as _; - spirv_tools::val::create(target_env).validate(spirv_tools::binary::to_binary(bytes)?, options) -} diff --git a/crates/rustc_codegen_spirv/src/link.rs b/crates/rustc_codegen_spirv/src/link.rs index 0ed9539188..ea47508e83 100644 --- a/crates/rustc_codegen_spirv/src/link.rs +++ b/crates/rustc_codegen_spirv/src/link.rs @@ -251,7 +251,7 @@ fn do_spirv_opt(sess: &Session, spv_binary: Vec, filename: &Path) -> Vec { @@ -201,29 +201,6 @@ OpDecorate %5 Binding 0 ); } -// NOTE(eddyb) we specifically run Vulkan validation here, as the default -// validation rules are more lax and don't require a `Block` decoration -// (`#[spirv(block)]` here) on `struct ShaderConstants`. -#[test] -fn push_constant_vulkan() { - val_vulkan( - r#" -#[derive(Copy, Clone)] -#[spirv(block)] -pub struct ShaderConstants { - pub width: u32, - pub height: u32, - pub time: f32, -} - -#[spirv(fragment)] -pub fn main(#[spirv(push_constant)] constants: &ShaderConstants) { - let _constants = *constants; -} -"#, - ); -} - #[test] fn unroll_loops() { dis_fn( diff --git a/crates/spirv-builder/src/test/mod.rs b/crates/spirv-builder/src/test/mod.rs index 4d819470c5..d51953c60a 100644 --- a/crates/spirv-builder/src/test/mod.rs +++ b/crates/spirv-builder/src/test/mod.rs @@ -99,19 +99,6 @@ fn val(src: &str) { build(src); } -/// While `val` runs baseline SPIR-V validation, for some tests we want the -/// stricter Vulkan validation (`vulkan1.2` specifically), which may produce -/// additional errors (such as missing Vulkan-specific decorations). -fn val_vulkan(src: &str) { - use rustc_codegen_spirv::{spirv_tools_validate as validate, SpirvToolsTargetEnv as TargetEnv}; - - let _lock = global_lock(); - let bytes = std::fs::read(build(src)).unwrap(); - if let Err(e) = validate(Some(TargetEnv::Vulkan_1_2), &bytes, None) { - panic!("Vulkan validation failed:\n{}", e.to_string()); - } -} - fn assert_str_eq(expected: &str, result: &str) { let expected = expected .split('\n') diff --git a/docs/src/testing.md b/docs/src/testing.md index 1977c62d8f..5e1b2c5245 100644 --- a/docs/src/testing.md +++ b/docs/src/testing.md @@ -39,5 +39,16 @@ cargo compiletest arch/u image The above command will only test `ui/arch/u_*.rs` and `ui/image/*.rs`, and skip everything else. You can also add `--bless` to update expected outputs, as well. +### Testing Different Environments + +You can test against multiple different SPIR-V environments with the +`--target-env` flag. By default it is set to `unknown`. + +```bash +cargo compiletest --target-env=vulkan1.1 +# You can also provide multiple values to test multiple environments +cargo compiletest --target-env=vulkan1.1,spv.1.3 +``` + [`compiletest`]: https://github.com/laumann/compiletest-rs [rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/tests/intro.html diff --git a/tests/src/main.rs b/tests/src/main.rs index f4b39663e5..59f1d6c3f2 100644 --- a/tests/src/main.rs +++ b/tests/src/main.rs @@ -17,12 +17,25 @@ struct Opt { #[structopt(long)] bless: bool, + /// The environment to compile to the SPIR-V tests. + #[structopt(long)] + target_env: Option, + /// Only run tests that match these filters #[structopt(name = "FILTER")] filters: Vec, } -const TARGET: &str = "spirv-unknown-unknown"; +impl Opt { + pub fn environments(&self) -> Vec { + match &self.target_env { + Some(env) => env.split(',').map(String::from).collect(), + None => vec!["unknown".into()], + } + } +} + +const TARGET_PREFIX: &str = "spirv-unknown-"; #[derive(Copy, Clone)] enum DepKind { @@ -38,10 +51,10 @@ impl DepKind { } } - fn target_dir_suffix(self) -> &'static str { + fn target_dir_suffix(self, target: &str) -> String { match self { - Self::SpirvLib => "spirv-unknown-unknown/debug/deps", - Self::ProcMacro => "debug/deps", + Self::SpirvLib => format!("{}/debug/deps", target), + Self::ProcMacro => "debug/deps".into(), } } } @@ -49,7 +62,7 @@ impl DepKind { fn main() { let opt = Opt::from_args(); - let tests_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let tests_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let workspace_root = tests_dir.parent().unwrap(); let original_target_dir = workspace_root.join("target"); let deps_target_dir = original_target_dir.join("compiletest-deps"); @@ -58,90 +71,97 @@ fn main() { // Pull in rustc_codegen_spirv as a dynamic library in the same way // spirv-builder does. let codegen_backend_path = find_rustc_codegen_spirv(); - let libs = build_deps(&deps_target_dir, &codegen_backend_path); - run_mode( - "ui", + let runner = Runner { opt, tests_dir, compiletest_build_dir, - &deps_target_dir, - &codegen_backend_path, - &libs, - ); + deps_target_dir, + codegen_backend_path, + }; + + runner.run_mode("ui"); } -// FIXME(eddyb) a bunch of these functions could be nicer if they were methods. - -/// Runs the given `mode` on the directory that matches that name, using the -/// backend provided by `codegen_backend_path`. -fn run_mode( - mode: &'static str, +struct Runner { opt: Opt, - tests_dir: &Path, + tests_dir: PathBuf, compiletest_build_dir: PathBuf, - deps_target_dir: &Path, - codegen_backend_path: &Path, - libs: &TestDeps, -) { - let mut config = compiletest::Config::default(); + deps_target_dir: PathBuf, + codegen_backend_path: PathBuf, +} - /// RUSTFLAGS passed to all test files. - fn test_rustc_flags( - codegen_backend_path: &Path, - deps: &TestDeps, - indirect_deps_dirs: &[&Path], - ) -> String { - [ - &*rust_flags(codegen_backend_path), - &*indirect_deps_dirs - .iter() - .map(|dir| format!("-L dependency={}", dir.display())) - .fold(String::new(), |a, b| b + " " + &a), - "--edition 2018", - &*format!("--extern noprelude:core={}", deps.core.display()), - &*format!( - "--extern noprelude:compiler_builtins={}", - deps.compiler_builtins.display() - ), - &*format!( - "--extern spirv_std_macros={}", - deps.spirv_std_macros.display() - ), - &*format!("--extern spirv_std={}", deps.spirv_std.display()), - &*format!("--extern glam={}", deps.glam.display()), - "--crate-type dylib", - "-Zunstable-options", - "-Zcrate-attr=no_std", - "-Zcrate-attr=feature(register_attr,asm)", - "-Zcrate-attr=register_attr(spirv)", - ] - .join(" ") +impl Runner { + /// Runs the given `mode` on the directory that matches that name, using the + /// backend provided by `codegen_backend_path`. + fn run_mode(&self, mode: &'static str) { + /// RUSTFLAGS passed to all test files. + fn test_rustc_flags( + codegen_backend_path: &Path, + deps: &TestDeps, + indirect_deps_dirs: &[&Path], + ) -> String { + [ + &*rust_flags(codegen_backend_path), + &*indirect_deps_dirs + .iter() + .map(|dir| format!("-L dependency={}", dir.display())) + .fold(String::new(), |a, b| b + " " + &a), + "--edition 2018", + &*format!("--extern noprelude:core={}", deps.core.display()), + &*format!( + "--extern noprelude:compiler_builtins={}", + deps.compiler_builtins.display() + ), + &*format!( + "--extern spirv_std_macros={}", + deps.spirv_std_macros.display() + ), + &*format!("--extern spirv_std={}", deps.spirv_std.display()), + &*format!("--extern glam={}", deps.glam.display()), + "--crate-type dylib", + "-Zunstable-options", + "-Zcrate-attr=no_std", + "-Zcrate-attr=feature(register_attr,asm)", + "-Zcrate-attr=register_attr(spirv)", + ] + .join(" ") + } + + for env in self.opt.environments() { + let target = format!("{}{}", TARGET_PREFIX, env); + let mut config = compiletest::Config::default(); + let libs = build_deps(&self.deps_target_dir, &self.codegen_backend_path, &target); + + let flags = test_rustc_flags( + &self.codegen_backend_path, + &libs, + &[ + &self + .deps_target_dir + .join(DepKind::SpirvLib.target_dir_suffix(&target)), + &self + .deps_target_dir + .join(DepKind::ProcMacro.target_dir_suffix(&target)), + ], + ); + + config.target_rustcflags = Some(flags); + config.mode = mode.parse().expect("Invalid mode"); + config.target = target; + config.src_base = self.tests_dir.join(mode); + config.build_base = self.compiletest_build_dir.clone(); + config.bless = self.opt.bless; + config.filters = self.opt.filters.clone(); + config.clean_rmeta(); + + compiletest::run_tests(&config); + } } - - let flags = test_rustc_flags( - codegen_backend_path, - libs, - &[ - &deps_target_dir.join(DepKind::SpirvLib.target_dir_suffix()), - &deps_target_dir.join(DepKind::ProcMacro.target_dir_suffix()), - ], - ); - - config.target_rustcflags = Some(flags); - config.mode = mode.parse().expect("Invalid mode"); - config.target = String::from(TARGET); - config.src_base = tests_dir.join(mode); - config.build_base = compiletest_build_dir; - config.bless = opt.bless; - config.filters = opt.filters; - config.clean_rmeta(); - - compiletest::run_tests(&config); } /// Runs the processes needed to build `spirv-std` & other deps. -fn build_deps(deps_target_dir: &Path, codegen_backend_path: &Path) -> TestDeps { +fn build_deps(deps_target_dir: &Path, codegen_backend_path: &Path, target: &str) -> TestDeps { // HACK(eddyb) this is only needed until we enable `resolver = "2"`, as the // old ("1") resolver has a bug where it picks up extra features based on the // current directory (and so we always set the working dir as a workaround). @@ -154,7 +174,7 @@ fn build_deps(deps_target_dir: &Path, codegen_backend_path: &Path) -> TestDeps { "-p", "compiletests-deps-helper", "-Zbuild-std=core", - &*format!("--target={}", TARGET), + &*format!("--target={}", target), ]) .arg("--target-dir") .arg(deps_target_dir) @@ -166,13 +186,23 @@ fn build_deps(deps_target_dir: &Path, codegen_backend_path: &Path) -> TestDeps { .and_then(map_status_to_result) .unwrap(); - let compiler_builtins = - find_lib(deps_target_dir, "compiler_builtins", DepKind::SpirvLib).unwrap(); - let core = find_lib(deps_target_dir, "core", DepKind::SpirvLib).unwrap(); - let spirv_std = find_lib(deps_target_dir, "spirv_std", DepKind::SpirvLib).unwrap(); - let glam = find_lib(deps_target_dir, "glam", DepKind::SpirvLib).unwrap(); - let spirv_std_macros = - find_lib(deps_target_dir, "spirv_std_macros", DepKind::ProcMacro).unwrap(); + let compiler_builtins = find_lib( + deps_target_dir, + "compiler_builtins", + DepKind::SpirvLib, + target, + ) + .unwrap(); + let core = find_lib(deps_target_dir, "core", DepKind::SpirvLib, target).unwrap(); + let spirv_std = find_lib(deps_target_dir, "spirv_std", DepKind::SpirvLib, target).unwrap(); + let glam = find_lib(deps_target_dir, "glam", DepKind::SpirvLib, target).unwrap(); + let spirv_std_macros = find_lib( + deps_target_dir, + "spirv_std_macros", + DepKind::ProcMacro, + target, + ) + .unwrap(); if [ &compiler_builtins, @@ -185,7 +215,7 @@ fn build_deps(deps_target_dir: &Path, codegen_backend_path: &Path) -> TestDeps { .any(|o| o.is_none()) { clean_deps(deps_target_dir); - build_deps(deps_target_dir, codegen_backend_path) + build_deps(deps_target_dir, codegen_backend_path, target) } else { TestDeps { core: core.unwrap(), @@ -215,12 +245,13 @@ fn find_lib( deps_target_dir: &Path, base: impl AsRef, dep_kind: DepKind, + target: &str, ) -> Result> { let base = base.as_ref(); let (expected_prefix, expected_extension) = dep_kind.prefix_and_extension(); let expected_name = format!("{}{}", expected_prefix, base.display()); - let dir = deps_target_dir.join(dep_kind.target_dir_suffix()); + let dir = deps_target_dir.join(dep_kind.target_dir_suffix(target)); let paths = std::fs::read_dir(dir)? .filter_map(Result::ok) diff --git a/tests/ui/image/fetch.rs b/tests/ui/image/fetch.rs index cd83fa85ba..df54d268e7 100644 --- a/tests/ui/image/fetch.rs +++ b/tests/ui/image/fetch.rs @@ -3,7 +3,10 @@ use spirv_std::{arch, Image2d}; #[spirv(fragment)] -pub fn main(#[spirv(uniform_constant)] image: &Image2d, output: &mut glam::Vec4) { +pub fn main( + #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image: &Image2d, + output: &mut glam::Vec4, +) { let texel = image.fetch(glam::IVec2::new(0, 1)); *output = texel; } diff --git a/tests/ui/image/issue_527.rs b/tests/ui/image/issue_527.rs new file mode 100644 index 0000000000..83e86df6af --- /dev/null +++ b/tests/ui/image/issue_527.rs @@ -0,0 +1,25 @@ +use glam::*; + +#[spirv(block)] +pub struct PointsBuffer { + points: [UVec2; 100], +} + +#[spirv(compute(threads(1)))] +pub fn main_cs( + #[spirv(global_invocation_id)] id: UVec3, + #[spirv(storage_buffer, descriptor_set = 0, binding = 0)] points_buffer: &mut PointsBuffer, + #[spirv(uniform_constant, descriptor_set = 1, binding = 1)] image: &spirv_std::StorageImage2d, +) { + unsafe { asm!("OpCapability StorageImageWriteWithoutFormat") }; + let position = id.xy(); + for i in 0..100usize { + let p0 = &points_buffer.points[i]; + let p1 = &points_buffer.points[i + 1]; + if p0.x == position.x && p1.y == position.y { + unsafe { + image.write(position, vec2(1.0, 0.0)); + }; + } + } +} diff --git a/tests/ui/image/read.rs b/tests/ui/image/read.rs index 849f5fd2f1..ae0eeeab1e 100644 --- a/tests/ui/image/read.rs +++ b/tests/ui/image/read.rs @@ -4,7 +4,11 @@ use spirv_std::{arch, StorageImage2d}; #[spirv(fragment)] -pub fn main(#[spirv(uniform_constant)] image: &StorageImage2d, output: &mut glam::Vec2) { +pub fn main( + #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image: &StorageImage2d, + output: &mut glam::Vec4, +) { + unsafe { asm!("OpCapability StorageImageReadWithoutFormat") }; let coords = image.read(glam::IVec2::new(0, 1)); *output = coords; } diff --git a/tests/ui/image/sample.rs b/tests/ui/image/sample.rs index 176ec08492..4b2f7298b4 100644 --- a/tests/ui/image/sample.rs +++ b/tests/ui/image/sample.rs @@ -5,10 +5,10 @@ use spirv_std::{arch, Cubemap, Image2d, Image2dArray, Sampler}; #[spirv(fragment)] pub fn main( - #[spirv(uniform_constant)] image2d: &Image2d, - #[spirv(uniform_constant)] image2d_array: &Image2dArray, - #[spirv(uniform_constant)] cubemap: &Cubemap, - #[spirv(uniform_constant)] sampler: &Sampler, + #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image2d: &Image2d, + #[spirv(uniform_constant, descriptor_set = 1, binding = 1)] image2d_array: &Image2dArray, + #[spirv(uniform_constant, descriptor_set = 2, binding = 2)] cubemap: &Cubemap, + #[spirv(uniform_constant, descriptor_set = 3, binding = 3)] sampler: &Sampler, output: &mut glam::Vec4, ) { let v2 = glam::Vec2::new(0.0, 1.0); diff --git a/tests/ui/image/sample_gradient.rs b/tests/ui/image/sample_gradient.rs index 24e20a2bec..2d4aa63420 100644 --- a/tests/ui/image/sample_gradient.rs +++ b/tests/ui/image/sample_gradient.rs @@ -5,10 +5,10 @@ use spirv_std::{arch, Cubemap, Image2d, Image2dArray, Sampler}; #[spirv(fragment)] pub fn main( - #[spirv(uniform_constant)] image2d: &Image2d, - #[spirv(uniform_constant)] image2d_array: &Image2dArray, - #[spirv(uniform_constant)] cubemap: &Cubemap, - #[spirv(uniform_constant)] sampler: &Sampler, + #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image2d: &Image2d, + #[spirv(uniform_constant, descriptor_set = 1, binding = 1)] image2d_array: &Image2dArray, + #[spirv(uniform_constant, descriptor_set = 2, binding = 2)] cubemap: &Cubemap, + #[spirv(uniform_constant, descriptor_set = 3, binding = 3)] sampler: &Sampler, output: &mut glam::Vec4, ) { let v2 = glam::Vec2::new(0.0, 1.0); diff --git a/tests/ui/image/sample_lod.rs b/tests/ui/image/sample_lod.rs index a547d5e370..129d540c7a 100644 --- a/tests/ui/image/sample_lod.rs +++ b/tests/ui/image/sample_lod.rs @@ -5,10 +5,10 @@ use spirv_std::{arch, Cubemap, Image2d, Image2dArray, Sampler}; #[spirv(fragment)] pub fn main( - #[spirv(uniform_constant)] image2d: &Image2d, - #[spirv(uniform_constant)] image2d_array: &Image2dArray, - #[spirv(uniform_constant)] cubemap: &Cubemap, - #[spirv(uniform_constant)] sampler: &Sampler, + #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image2d: &Image2d, + #[spirv(uniform_constant, descriptor_set = 1, binding = 1)] image2d_array: &Image2dArray, + #[spirv(uniform_constant, descriptor_set = 2, binding = 2)] cubemap: &Cubemap, + #[spirv(uniform_constant, descriptor_set = 3, binding = 3)] sampler: &Sampler, output: &mut glam::Vec4, ) { let v2 = glam::Vec2::new(0.0, 1.0); diff --git a/tests/ui/image/write.rs b/tests/ui/image/write.rs index e2cf172446..e38858798c 100644 --- a/tests/ui/image/write.rs +++ b/tests/ui/image/write.rs @@ -4,8 +4,12 @@ use spirv_std::{arch, StorageImage2d}; #[spirv(fragment)] -pub fn main(texels: glam::Vec2, #[spirv(uniform_constant)] image: &StorageImage2d) { +pub fn main( + texels: glam::Vec2, + #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image: &StorageImage2d, +) { unsafe { + asm!("OpCapability StorageImageWriteWithoutFormat"); image.write(glam::UVec2::new(0, 1), texels); } } diff --git a/tests/ui/storage_class/push_constant.rs b/tests/ui/storage_class/push_constant.rs index 3248d14b74..1605e6d58f 100644 --- a/tests/ui/storage_class/push_constant.rs +++ b/tests/ui/storage_class/push_constant.rs @@ -1,12 +1,10 @@ // Test that using push constants work. -// NOTE(eddyb) this won't pass Vulkan validation (see `push_constant_vulkan`), -// but should still pass the baseline SPIR-V validation. - // build-pass - use spirv_std as _; #[derive(Copy, Clone)] +// `Block` decoration is required for push constants when compiling for Vulkan. +#[cfg_attr(not(target_env = "unknown"), spirv(block))] pub struct ShaderConstants { pub width: u32, pub height: u32,