Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for multiple target environments #533

Merged
merged 4 commits into from
Mar 29, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 19 additions & 7 deletions crates/rustc_codegen_spirv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand Down Expand Up @@ -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,
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rustc_codegen_spirv/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,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());
Expand Down
25 changes: 1 addition & 24 deletions crates/spirv-builder/src/test/basic.rs
Original file line number Diff line number Diff line change
@@ -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> {
Expand Down Expand Up @@ -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(
XAMPPRocky marked this conversation as resolved.
Show resolved Hide resolved
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(
Expand Down
13 changes: 0 additions & 13 deletions crates/spirv-builder/src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
XAMPPRocky marked this conversation as resolved.
Show resolved Hide resolved

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')
Expand Down
11 changes: 11 additions & 0 deletions docs/src/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading