Add support for multiple target environments (#533)

* Add support for multiple target environments

* updates from code review

* Update lib.rs

.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

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<spirv_tools::TargetEnv>) -> 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<Target> {
         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<dyn CodegenBackend> {
 
     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<spirv_tools::TargetEnv>,
-    bytes: &[u8],
-    options: Option<spirv_tools::val::ValidatorOptions>,
-) -> 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)
-}

crates/rustc_codegen_spirv/src/link.rs

@@ -251,7 +251,7 @@ fn do_spirv_opt(sess: &Session, spv_binary: Vec<u32>, filename: &Path) -> Vec<u3
 fn do_spirv_val(sess: &Session, spv_binary: &[u32], filename: &Path) {
     use spirv_tools::val::{self, Validator};
 
-    let validator = val::create(None);
+    let validator = val::create(sess.target.options.env.parse().ok());
 
     if let Err(e) = validator.validate(spv_binary, None) {
         let mut err = sess.struct_err(&e.to_string());

crates/spirv-builder/src/test/basic.rs

@@ -1,4 +1,4 @@
-use super::{dis_entry_fn, dis_fn, dis_globals, val, val_vulkan};
+use super::{dis_entry_fn, dis_fn, dis_globals, val};
 use std::ffi::OsStr;
 
 struct SetEnvVar<'a> {
@@ -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(

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')

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

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<String>,
+
     /// Only run tests that match these filters
     #[structopt(name = "FILTER")]
     filters: Vec<String>,
 }
 
-const TARGET: &str = "spirv-unknown-unknown";
+impl Opt {
+    pub fn environments(&self) -> Vec<String> {
+        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,34 +71,30 @@ 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,
+}
 
+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,
@@ -119,29 +128,40 @@ fn run_mode(
             .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(
-        codegen_backend_path,
-        libs,
+                &self.codegen_backend_path,
+                &libs,
                 &[
-            &deps_target_dir.join(DepKind::SpirvLib.target_dir_suffix()),
-            &deps_target_dir.join(DepKind::ProcMacro.target_dir_suffix()),
+                    &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 = 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.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);
+        }
+    }
 }
 
 /// 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<Path>,
     dep_kind: DepKind,
+    target: &str,
 ) -> Result<Option<PathBuf>> {
     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)

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;
 }

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));
+            };
+        }
+    }
+}

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;
 }

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);

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);

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);

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);
     }
 }

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,