updates from code review

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

@@ -1,4 +1,4 @@
-use super::{dis_fn, dis_globals, val, val_vulkan};
+use super::{dis_fn, dis_globals, val};
 use std::ffi::OsStr;
 
 struct SetEnvVar<'a> {
@@ -200,29 +200,6 @@ OpDecorate %4 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')

tests/src/main.rs

@@ -17,13 +17,13 @@ struct Opt {
  #[structopt(long)]
  bless: bool,
 
- /// Only run tests that match these filters
- #[structopt(name = "FILTER")]
- filters: Vec<String>,
-
  /// 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>,
 }
 
 impl Opt {
@@ -83,8 +83,6 @@ fn main() {
  runner.run_mode("ui");
 }
 
-// FIXME(eddyb) a bunch of these functions could be nicer if they were methods.
-
 struct Runner {
  opt: Opt,
  tests_dir: PathBuf,

tests/ui/image/fetch.rs

@@ -3,7 +3,10 @@
 use spirv_std::{arch, Image2d};
 
 #[spirv(fragment)]
-pub fn main(#[spirv(uniform_constant, descriptor_set = 0, binding = 0)] 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

@@ -7,12 +7,9 @@ pub struct PointsBuffer {
 
 #[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,
+ #[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();
@@ -21,9 +18,8 @@ pub fn main_cs(
  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));
+ image.write(position, vec2(1.0, 0.0));
  };
  }
  }
 }
-

tests/ui/image/read.rs

@@ -4,7 +4,10 @@
 use spirv_std::{arch, StorageImage2d};
 
 #[spirv(fragment)]
-pub fn main(#[spirv(uniform_constant, descriptor_set = 0, binding = 0)] image: &StorageImage2d, output: &mut glam::Vec4) {
+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/write.rs

@@ -4,7 +4,10 @@
 use spirv_std::{arch, StorageImage2d};
 
 #[spirv(fragment)]
-pub fn main(texels: glam::Vec2, #[spirv(uniform_constant, descriptor_set = 0, binding = 0)] 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,13 +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)]
-#[spirv(block)]
+// `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,