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 spirv-val flags to spirv-builder #635

Merged
merged 1 commit into from
May 31, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
33 changes: 33 additions & 0 deletions crates/rustc_codegen_spirv/src/codegen_cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,14 @@ pub struct CodegenArgs {
pub disassemble_fn: Option<String>,
pub disassemble_entry: Option<String>,
pub disassemble_globals: bool,

// spirv-val flags
pub relax_struct_store: bool,
pub relax_logical_pointer: bool,
pub relax_block_layout: Option<bool>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is basically a tri-state? It stands out a bit here, maybe deserves a comment (although this is kind of a deep location).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's... a little weird? Even weirder is that I don't think the CLI of spirv-val exposes this tri-state, it's only Some(true) and None. I thiiiink the tri-state is:

  • Some(true) -> force relax-block-layout on
  • Some(false) -> force relax-block-layout off
  • None -> relax-block-layout is on if vulkan version >=1.1, off if ==1.0

pub uniform_buffer_standard_layout: bool,
pub scalar_block_layout: bool,
pub skip_block_layout: bool,
}

impl CodegenArgs {
Expand Down Expand Up @@ -305,19 +313,44 @@ impl CodegenArgs {
"NAME",
);
opts.optflagopt("", "disassemble-globals", "print globals to stderr", "");

opts.optflagopt("", "relax-struct-store", "Allow store from one struct type to a different type with compatible layout and members.", "");
opts.optflagopt("", "relax-logical-pointer", "Allow allocating an object of a pointer type and returning a pointer value from a function in logical addressing mode", "");
opts.optflagopt("", "relax-block-layout", "Enable VK_KHR_relaxed_block_layout when checking standard uniform, storage buffer, and push constant layouts. This is the default when targeting Vulkan 1.1 or later.", "");
opts.optflagopt("", "uniform-buffer-standard-layout", "Enable VK_KHR_uniform_buffer_standard_layout when checking standard uniform buffer layouts.", "");
opts.optflagopt("", "scalar-block-layout", "Enable VK_EXT_scalar_block_layout when checking standard uniform, storage buffer, and push constant layouts. Scalar layout rules are more permissive than relaxed block layout so in effect this will override the --relax-block-layout option.", "");
opts.optflagopt("", "skip-block-layout", "Skip checking standard uniform/storage buffer layout. Overrides any --relax-block-layout or --scalar-block-layout option.", "");

let matches = opts.parse(args)?;
let module_output_type =
matches.opt_get_default("module-output", ModuleOutputType::Single)?;
let disassemble = matches.opt_present("disassemble");
let disassemble_fn = matches.opt_str("disassemble-fn");
let disassemble_entry = matches.opt_str("disassemble-entry");
let disassemble_globals = matches.opt_present("disassemble-globals");

let relax_struct_store = matches.opt_present("relax-struct-store");
let relax_logical_pointer = matches.opt_present("relax-logical-pointer");
let relax_block_layout = matches.opt_present("relax-block-layout");
let uniform_buffer_standard_layout = matches.opt_present("uniform-buffer-standard-layout");
let scalar_block_layout = matches.opt_present("scalar-block-layout");
let skip_block_layout = matches.opt_present("skip-block-layout");

let relax_block_layout = if relax_block_layout { Some(true) } else { None };

Ok(Self {
module_output_type,
disassemble,
disassemble_fn,
disassemble_entry,
disassemble_globals,

relax_struct_store,
relax_logical_pointer,
relax_block_layout,
uniform_buffer_standard_layout,
scalar_block_layout,
skip_block_layout,
})
}

Expand Down
54 changes: 42 additions & 12 deletions crates/rustc_codegen_spirv/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ fn link_exe(
linker::LinkResult::SingleModule(spv_binary) => {
let mut module_filename = out_dir;
module_filename.push("module");
post_link_single_module(sess, spv_binary.assemble(), &module_filename);
post_link_single_module(sess, &cg_args, spv_binary.assemble(), &module_filename);
cg_args.do_disassemble(&spv_binary);
let module_result = ModuleResult::SingleModule(module_filename);
CompileResult {
Expand All @@ -167,7 +167,7 @@ fn link_exe(
for (name, spv_binary) in map {
let mut module_filename = out_dir.clone();
module_filename.push(sanitize_filename::sanitize(&name));
post_link_single_module(sess, spv_binary.assemble(), &module_filename);
post_link_single_module(sess, &cg_args, spv_binary.assemble(), &module_filename);
hashmap.insert(name, module_filename);
}
let module_result = ModuleResult::MultiModule(hashmap);
Expand All @@ -191,24 +191,46 @@ fn entry_points(module: &rspirv::dr::Module) -> Vec<String> {
.collect()
}

fn post_link_single_module(sess: &Session, spv_binary: Vec<u32>, out_filename: &Path) {
fn post_link_single_module(
sess: &Session,
cg_args: &crate::codegen_cx::CodegenArgs,
spv_binary: Vec<u32>,
out_filename: &Path,
) {
if let Ok(ref path) = std::env::var("DUMP_POST_LINK") {
File::create(path)
.unwrap()
.write_all(spirv_tools::binary::from_binary(&spv_binary))
.unwrap();
}

let val_options = spirv_tools::val::ValidatorOptions {
relax_struct_store: cg_args.relax_struct_store,
relax_logical_pointer: cg_args.relax_logical_pointer,
before_legalization: false,
relax_block_layout: cg_args.relax_block_layout,
uniform_buffer_standard_layout: cg_args.uniform_buffer_standard_layout,
scalar_block_layout: cg_args.scalar_block_layout,
skip_block_layout: cg_args.skip_block_layout,
max_limits: vec![],
};
let opt_options = spirv_tools::opt::Options {
validator_options: Some(val_options.clone()),
max_id_bound: None,
preserve_bindings: false,
preserve_spec_constants: false,
};

let spv_binary = if sess.opts.optimize != OptLevel::No || sess.opts.debuginfo == DebugInfo::None
{
let _timer = sess.timer("link_spirv_opt");
do_spirv_opt(sess, spv_binary, out_filename)
do_spirv_opt(sess, spv_binary, out_filename, opt_options)
} else {
spv_binary
};

if env::var("NO_SPIRV_VAL").is_err() {
do_spirv_val(sess, &spv_binary, out_filename);
do_spirv_val(sess, &spv_binary, out_filename, val_options);
}

{
Expand All @@ -225,13 +247,18 @@ fn post_link_single_module(sess: &Session, spv_binary: Vec<u32>, out_filename: &
}
}

fn do_spirv_opt(sess: &Session, spv_binary: Vec<u32>, filename: &Path) -> Vec<u32> {
fn do_spirv_opt(
sess: &Session,
spv_binary: Vec<u32>,
filename: &Path,
options: spirv_tools::opt::Options,
) -> Vec<u32> {
use spirv_tools::{
error,
opt::{self, Optimizer},
};

let mut optimizer = opt::create(None);
let mut optimizer = opt::create(sess.target.options.env.parse().ok());

match sess.opts.optimize {
OptLevel::No => {}
Expand Down Expand Up @@ -266,9 +293,7 @@ fn do_spirv_opt(sess: &Session, spv_binary: Vec<u32>, filename: &Path) -> Vec<u3
err.note(&format!("module `{}`", filename.display()));
err.emit();
},
// We currently run the validator separately after optimization or even
// if we don't run optimization, the default options don't run the validator
None,
Some(options),
);

match result {
Expand All @@ -283,12 +308,17 @@ 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) {
fn do_spirv_val(
sess: &Session,
spv_binary: &[u32],
filename: &Path,
options: spirv_tools::val::ValidatorOptions,
) {
use spirv_tools::val::{self, Validator};

let validator = val::create(sess.target.options.env.parse().ok());

if let Err(e) = validator.validate(spv_binary, None) {
if let Err(e) = validator.validate(spv_binary, Some(options)) {
let mut err = sess.struct_err(&e.to_string());
err.note("spirv-val failed");
err.note(&format!("module `{}`", filename.display()));
Expand Down
100 changes: 96 additions & 4 deletions crates/spirv-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use std::collections::HashMap;
use std::env;
use std::error::Error;
use std::fmt;
use std::fmt::Write;
use std::fs::File;
use std::io::BufReader;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -115,6 +116,14 @@ pub struct SpirvBuilder {
multimodule: bool,
capabilities: Vec<Capability>,
extensions: Vec<String>,

// spirv-val flags
pub relax_struct_store: bool,
pub relax_logical_pointer: bool,
pub relax_block_layout: bool,
pub uniform_buffer_standard_layout: bool,
pub scalar_block_layout: bool,
pub skip_block_layout: bool,
}

impl SpirvBuilder {
Expand All @@ -128,6 +137,13 @@ impl SpirvBuilder {
multimodule: false,
capabilities: Vec::new(),
extensions: Vec::new(),

relax_struct_store: false,
relax_logical_pointer: false,
relax_block_layout: false,
uniform_buffer_standard_layout: false,
scalar_block_layout: false,
skip_block_layout: false,
}
}

Expand Down Expand Up @@ -172,6 +188,48 @@ impl SpirvBuilder {
self
}

/// Allow store from one struct type to a different type with compatible layout and members.
pub fn relax_struct_store(mut self, v: bool) -> Self {
self.relax_struct_store = v;
self
}

/// Allow allocating an object of a pointer type and returning a pointer value from a function
/// in logical addressing mode
pub fn relax_logical_pointer(mut self, v: bool) -> Self {
self.relax_logical_pointer = v;
self
}

/// Enable `VK_KHR_relaxed_block_layout` when checking standard uniform, storage buffer, and
/// push constant layouts. This is the default when targeting Vulkan 1.1 or later.
pub fn relax_block_layout(mut self, v: bool) -> Self {
self.relax_block_layout = v;
self
}

/// Enable `VK_KHR_uniform_buffer_standard_layout` when checking standard uniform buffer
/// layouts.
pub fn uniform_buffer_standard_layout(mut self, v: bool) -> Self {
self.uniform_buffer_standard_layout = v;
self
}

/// Enable `VK_EXT_scalar_block_layout` when checking standard uniform, storage buffer, and
/// push constant layouts. Scalar layout rules are more permissive than relaxed block layout so
/// in effect this will override the --relax-block-layout option.
pub fn scalar_block_layout(mut self, v: bool) -> Self {
self.scalar_block_layout = v;
self
}

/// Skip checking standard uniform/storage buffer layout. Overrides any --relax-block-layout or
/// --scalar-block-layout option.
pub fn skip_block_layout(mut self, v: bool) -> Self {
self.skip_block_layout = v;
self
}

/// Builds the module. If `print_metadata` is true, you usually don't have to inspect the path
/// in the result, as the environment variable for the path to the module will already be set.
pub fn build(self) -> Result<CompileResult, SpirvBuilderError> {
Expand Down Expand Up @@ -245,10 +303,44 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
// rustc expects a full path, instead of a filename looked up via LD_LIBRARY_PATH, so we need
// to copy cargo's understanding of library lookup and find the library and its full path.
let rustc_codegen_spirv = find_rustc_codegen_spirv();
let llvm_args = builder
.multimodule
.then(|| " -C llvm-args=--module-output=multiple")
.unwrap_or_default();

let mut llvm_args = Vec::new();
if builder.multimodule {
llvm_args.push("--module-output=multiple");
}
if builder.relax_struct_store {
llvm_args.push("--relax-struct-store");
}
if builder.relax_logical_pointer {
llvm_args.push("--relax-logical-pointer");
}
if builder.relax_block_layout {
llvm_args.push("--relax-block-layout");
}
if builder.uniform_buffer_standard_layout {
llvm_args.push("--uniform-buffer-standard-layout");
}
if builder.scalar_block_layout {
llvm_args.push("--scalar-block-layout");
}
if builder.skip_block_layout {
llvm_args.push("--skip-block-layout");
}

let llvm_args = if llvm_args.is_empty() {
String::new()
} else {
// Cargo's handling of RUSTFLAGS is a little cursed. -Cllvm-args is documented as "The list
// must be separated by spaces", but if we set RUSTFLAGS='-C llvm-args="--foo --bar"', then
// cargo will pass -C 'llvm-args="--foo' '--bar"' to rustc. Like, really? c'mon.
// Thankfully, passing -C llvm-args multiple times appends to a list, instead of
// overwriting.
let mut result = String::new();
for arg in llvm_args {
write!(result, " -C llvm-args={}", arg).unwrap();
}
result
};

let mut target_features = Vec::new();

Expand Down