Get tests to work both with installed and compiled tools

Author: Jake-Shadle

.github/workflows/ci.yaml

@@ -58,6 +58,7 @@ jobs:
       - run: rustup component add rustfmt clippy rust-src rustc-dev llvm-tools-preview
       - run: cargo fmt --all -- --check
       - run: cargo clippy --workspace --all-targets -- -D warnings
+
   cargo-deny:
     runs-on: ubuntu-latest
     steps:

rustc_codegen_spirv/src/link.rs

@@ -155,15 +155,12 @@ fn link_exe(
 }
 
 fn do_spirv_opt(sess: &Session, spv_binary: Vec<u32>, filename: &Path) -> Vec<u32> {
-    use spirv_tools::{error, opt::{
-        self,
-        Optimizer,
-    }};
+    use spirv_tools::{
+        error,
+        opt::{self, Optimizer},
+    };
 
-    #[cfg(feature = "use-compiled-tools")]
-    let mut optimizer = opt::compiled::CompiledOptimizer;
-    #[cfg(all(feature = "use-installed-tools", not(feature = "use-compiled-tools")))]
-    let mut optimizer = opt::tool::ToolOptimizer;
+    let mut optimizer = opt::create(None);
 
     optimizer
         .register_size_passes()
@@ -172,7 +169,7 @@ fn do_spirv_opt(sess: &Session, spv_binary: Vec<u32>, filename: &Path) -> Vec<u3
 
     let result = optimizer.optimize(
         &spv_binary,
-        &mut |msg: error::Message<'_>| {
+        &mut |msg: error::Message| {
             use error::MessageLevel as Level;
 
             // TODO: Adds spans here? Not sure how useful with binary, but maybe?
@@ -203,9 +200,9 @@ 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;
+    use spirv_tools::val::{self, Validator};
 
-    let validator = val::Validator::new(spirv_tools::TargetEnv::default());
+    let validator = val::create(None);
 
     if validator.validate(spv_binary, None).is_err() {
         let mut err = sess.struct_err("error occurred during validation");

rustc_codegen_spirv/src/linker/test.rs

@@ -13,9 +13,9 @@ impl<'a> std::fmt::Debug for PrettyString<'a> {
 }
 
 fn assemble_spirv(spirv: &str) -> Result<Vec<u8>> {
-    use spirv_tools::assembler;
+    use spirv_tools::assembler::{self, Assembler};
 
-    let assembler = assembler::Assembler::new(spirv_tools::default_target_env());
+    let assembler = assembler::create(None);
 
     let spv_binary = assembler.assemble(spirv, assembler::AssemblerOptions::default())?;
 
@@ -25,10 +25,12 @@ fn assemble_spirv(spirv: &str) -> Result<Vec<u8>> {
 
 #[allow(unused)]
 fn validate(spirv: &[u32]) {
-    let validator = spirv_tools::val::Validator::new(spirv_tools::default_target_env());
+    use spirv_tools::val::{self, Validator};
+
+    let validator = val::create(None);
 
     validator
-        .validate(spirv, &spirv_tools::val::ValidatorOptions::default())
+        .validate(spirv, None)
         .expect("validation error occurred");
 }
 

spirv-builder/Cargo.toml

@@ -5,8 +5,6 @@ authors = ["Embark <[email protected]>"]
 edition = "2018"
 license = "MIT OR Apache-2.0"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
 [dependencies]
 memchr = "2.3"
 raw-string = "0.3.5"

spirv-tools/examples/as.rs

@@ -39,7 +39,8 @@ fn main() {
         preserve_numeric_ids: args.preserve_ids,
     };
 
-    let assembler = assembler::compiled::CompiledAssembler::default();
+    let assembler =
+        assembler::compiled::CompiledAssembler::with_env(args.target_env.unwrap_or_default());
 
     match assembler.assemble(&contents, assembler_opts) {
         Ok(binary) => {

spirv-tools/src/assembler.rs

@@ -33,4 +33,18 @@ pub trait Assembler: Default {
         text: &str,
         options: AssemblerOptions,
     ) -> Result<crate::binary::Binary, crate::error::Error>;
-}
+}
+
+pub fn create(te: Option<crate::TargetEnv>) -> impl Assembler {
+    let target_env = te.unwrap_or_default();
+
+    #[cfg(feature = "use-compiled")]
+    {
+        compiled::CompiledAssembler::with_env(target_env)
+    }
+
+    #[cfg(all(feature = "use-installed", not(feature = "use-compiled")))]
+    {
+        tool::ToolAssembler::with_env(target_env)
+    }
+}

spirv-tools/src/assembler/compiled.rs

@@ -44,7 +44,8 @@ impl Assembler for CompiledAssembler {
                         });
                     }
 
-                    Ok(crate::binary::Binary::External(crate::binary::external::ExternalBinary::new(binary)))
+                    let bin = crate::binary::external::ExternalBinary::new(binary);
+                    Ok(crate::binary::Binary::External(bin))
                 }
                 other => Err(crate::error::Error {
                     inner: other,

spirv-tools/src/assembler/tool.rs

@@ -6,9 +6,7 @@ use super::Assembler;
 
 impl Assembler for ToolAssembler {
     fn with_env(target_env: crate::TargetEnv) -> Self {
-        Self {
-            target_env,
-        }
+        Self { target_env }
     }
 
     fn assemble(
@@ -25,7 +23,8 @@ impl Assembler for ToolAssembler {
 
         cmd.arg("-o").arg("-");
 
-        let cmd_output = crate::cmd::exec(cmd, Some(text.as_bytes()), crate::cmd::Output::Retrieve)?;
+        let cmd_output =
+            crate::cmd::exec(cmd, Some(text.as_bytes()), crate::cmd::Output::Retrieve)?;
 
         use std::convert::TryFrom;
         crate::binary::Binary::try_from(cmd_output.binary)

spirv-tools/src/cmd.rs

@@ -21,22 +21,26 @@ impl From<CmdError> for crate::error::Error {
         use crate::SpirvResult;
 
         match ce {
-            CmdError::BinaryNotFound(e) => {
-                Self {
-                    inner: SpirvResult::Unsupported,
-                    diagnostic: Some(format!("failed spawn executable: {}", e).into()),
-                }
-            }
-            CmdError::Io(e) => {
-                Self {
-                    inner: SpirvResult::EndOfStream,
-                    diagnostic: Some(format!("i/o error occurred communicating with executable: {}", e).into()),
-                }
-            }
-            CmdError::ToolErrors { exit_code: _, messages } => {
+            CmdError::BinaryNotFound(e) => Self {
+                inner: SpirvResult::Unsupported,
+                diagnostic: Some(format!("failed spawn executable: {}", e).into()),
+            },
+            CmdError::Io(e) => Self {
+                inner: SpirvResult::EndOfStream,
+                diagnostic: Some(
+                    format!("i/o error occurred communicating with executable: {}", e).into(),
+                ),
+            },
+            CmdError::ToolErrors {
+                exit_code: _,
+                messages,
+            } => {
                 // The C API just puts the last message as the diagnostic, so just do the
                 // same for now
-                let diagnostic = messages.into_iter().last().map(crate::error::Diagnostic::from);
+                let diagnostic = messages
+                    .into_iter()
+                    .last()
+                    .map(crate::error::Diagnostic::from);
 
                 Self {
                     inner: SpirvResult::InternalError, // this isn't really correct
@@ -172,14 +176,17 @@ pub fn exec(
         }
 
         if retrieve_output {
+            // Handle case where there is a '\n' in the stream, but it's not the
+            // end of an output message
+            if !maybe_msg || messages.is_empty() && !binary.is_empty() {
+                binary.push(b'\n');
+            }
+
             binary.extend_from_slice(line);
         }
 
         maybe_msg = false;
     }
 
-    Ok(CmdOutput {
-        binary,
-        messages,
-    })
+    Ok(CmdOutput { binary, messages })
 }

spirv-tools/src/error.rs

@@ -1,7 +1,7 @@
 use spirv_tools_sys::{diagnostics, shared};
 
-pub use shared::SpirvResult;
 pub use diagnostics::MessageLevel;
+pub use shared::SpirvResult;
 
 #[derive(Debug, PartialEq)]
 pub struct Error {
@@ -66,7 +66,7 @@ impl std::convert::TryFrom<*mut diagnostics::Diagnostic> for Diagnostic {
     }
 }
 
-impl From<String> for Diagnostic{
+impl From<String> for Diagnostic {
     fn from(message: String) -> Self {
         Self {
             line: 0,
@@ -90,6 +90,7 @@ impl From<Message> for Diagnostic {
     }
 }
 
+#[derive(Debug)]
 pub struct Message {
     pub level: MessageLevel,
     pub source: Option<String>,
@@ -100,6 +101,7 @@ pub struct Message {
 }
 
 impl Message {
+    #[cfg(feature = "use-installed")]
     pub(crate) fn fatal(message: String) -> Self {
         Self {
             level: MessageLevel::Fatal,
@@ -133,7 +135,11 @@ impl Message {
 
             Self {
                 level,
-                source: if source.is_empty() { None } else { Some(source.into_owned()) },
+                source: if source.is_empty() {
+                    None
+                } else {
+                    Some(source.into_owned())
+                },
                 line,
                 column,
                 index,
@@ -142,6 +148,7 @@ impl Message {
         }
     }
 
+    #[cfg(feature = "use-installed")]
     pub(crate) fn parse(s: &str) -> Option<Self> {
         s.find(": ")
             .and_then(|i| {

spirv-tools/src/opt.rs

@@ -22,7 +22,7 @@ pub struct Options {
 
 pub trait Optimizer {
     fn with_env(target_env: crate::TargetEnv) -> Self;
-    
+
     fn optimize<MC: crate::error::MessageCallback>(
         &self,
         input: &[u32],
@@ -50,3 +50,17 @@ pub trait Optimizer {
     /// from time to time.
     fn register_hlsl_legalization_passes(&mut self) -> &mut Self;
 }
+
+pub fn create(te: Option<crate::TargetEnv>) -> impl Optimizer {
+    let target_env = te.unwrap_or_default();
+
+    #[cfg(feature = "use-compiled")]
+    {
+        compiled::CompiledOptimizer::with_env(target_env)
+    }
+
+    #[cfg(all(feature = "use-installed", not(feature = "use-compiled")))]
+    {
+        tool::ToolOptimizer::with_env(target_env)
+    }
+}

spirv-tools/src/opt/compiled.rs

@@ -44,20 +44,20 @@ impl Drop for Options {
     }
 }
 
-pub struct Optimizer {
+pub struct CompiledOptimizer {
     inner: *mut opt::Optimizer,
 }
 
-impl Optimizer {
-    #[inline]
-    pub fn new(target: crate::TargetEnv) -> Self {
+use super::Optimizer;
+
+impl Optimizer for CompiledOptimizer {
+    fn with_env(target: crate::TargetEnv) -> Self {
         Self {
             inner: unsafe { opt::optimizer_create(target) },
         }
     }
 
-    #[inline]
-    pub fn optimize<MC: error::MessageCallback>(
+    fn optimize<MC: error::MessageCallback>(
         &self,
         input: &[u32],
         msg_callback: &mut MC,
@@ -122,7 +122,9 @@ impl Optimizer {
                         });
                     }
 
-                    Ok(crate::binary::Binary::External(crate::binary::external::ExternalBinary::new(binary)))
+                    Ok(crate::binary::Binary::External(
+                        crate::binary::external::ExternalBinary::new(binary),
+                    ))
                 }
                 other => Err(error::Error {
                     inner: other,
@@ -134,7 +136,7 @@ impl Optimizer {
 
     /// Register a single pass with the the optimizer.
     #[inline]
-    pub fn register_pass(&mut self, pass: super::Passes) -> &mut Self {
+    fn register_pass(&mut self, pass: super::Passes) -> &mut Self {
         unsafe { opt::optimizer_register_pass(self.inner, pass) }
         self
     }
@@ -143,7 +145,7 @@ impl Optimizer {
     /// This sequence of passes is subject to constant review and will change
     /// from time to time.
     #[inline]
-    pub fn register_performance_passes(&mut self) -> &mut Self {
+    fn register_performance_passes(&mut self) -> &mut Self {
         unsafe { opt::optimizer_register_performance_passes(self.inner) }
         self
     }
@@ -152,7 +154,7 @@ impl Optimizer {
     /// This sequence of passes is subject to constant review and will change
     /// from time to time.
     #[inline]
-    pub fn register_size_passes(&mut self) -> &mut Self {
+    fn register_size_passes(&mut self) -> &mut Self {
         unsafe { opt::optimizer_register_size_passes(self.inner) }
         self
     }
@@ -184,19 +186,19 @@ impl Optimizer {
     /// This sequence of passes is subject to constant review and will change
     /// from time to time.
     #[inline]
-    pub fn register_hlsl_legalization_passes(&mut self) -> &mut Self {
+    fn register_hlsl_legalization_passes(&mut self) -> &mut Self {
         unsafe { opt::optimizer_register_hlsl_legalization_passes(self.inner) }
         self
     }
 }
 
-impl Default for Optimizer {
+impl Default for CompiledOptimizer {
     fn default() -> Self {
-        Self::new(crate::TargetEnv::default())
+        Self::with_env(crate::TargetEnv::default())
     }
 }
 
-impl Drop for Optimizer {
+impl Drop for CompiledOptimizer {
     #[inline]
     fn drop(&mut self) {
         unsafe { opt::optimizer_destroy(self.inner) }