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.

Sign up for GitHub

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

Speed up Naga's cargo xtask validate wgsl from 12s to 0.8s #4871

Merged
merged 3 commits into from
Dec 24, 2023
Merged

Speed up Naga's cargo xtask validate wgsl from 12s to 0.8s #4871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cfb731
[naga-cli] Abstract out parsing of input file.
jimblandy Dec 12, 2023
2e3c6c8
[naga-cli] Add `--bulk-validate` option.
jimblandy Dec 12, 2023
05f1fb9
[naga xtask] Use naga-cli's --bulk-validate option.
jimblandy Dec 12, 2023
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: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ Wgpu now exposes backend feature for the Direct3D 12 (`dx12`) and Metal (`metal`

- Naga constant evaluation can now process binary operators whose operands are both vectors. By @jimblandy in [#4861](https://github.com/gfx-rs/wgpu/pull/4861).

- Add `--bulk-validate` option to Naga CLI. By @jimblandy in [#4871](https://github.com/gfx-rs/wgpu/pull/4871).

### Changes

- Arcanization of wgpu core resources: By @gents83 in [#3626](https://github.com/gfx-rs/wgpu/pull/3626) and thanks also to @jimblandy, @nical, @Wumpf, @Elabajaba & @cwfitzgerald
Expand Down
236 changes: 158 additions & 78 deletions naga-cli/src/bin/naga.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ struct Args {
#[argh(option)]
before_compaction: Option<String>,

/// bulk validation mode: all filenames are inputs to read and validate.
#[argh(switch)]
bulk_validate: bool,

/// show version
#[argh(switch)]
version: bool,
Expand All @@ -105,6 +109,8 @@ struct Args {
///
/// The rest arguments are the output files. If not specified, only
/// validation will be performed.
///
/// In bulk validation mode, these are all input files to be validated.
#[argh(positional)]
files: Vec<String>,
}
Expand Down Expand Up @@ -236,27 +242,16 @@ impl std::error::Error for CliError {}
fn run() -> Result<(), Box<dyn std::error::Error>> {
env_logger::init();

// Initialize default parameters
//TODO: read the parameters from RON?
let mut params = Parameters::default();

// Parse commandline arguments
let args: Args = argh::from_env();
if args.version {
println!("{}", env!("CARGO_PKG_VERSION"));
return Ok(());
}
let (input_path, input) = if let Some(path) = args.files.first() {
let path = Path::new(path);
(path, fs::read(path)?)
} else if let Some(path) = &args.stdin_file_path {
let mut input = vec![];
std::io::stdin().lock().read_to_end(&mut input)?;
(Path::new(path), input)
} else {
return Err(CliError("Input file path is not specified").into());
};
let output_paths = args.files.get(1..).unwrap_or(&[]);

// Initialize default parameters
//TODO: read the parameters from RON?
let mut params = Parameters::default();
ErichDonGubler marked this conversation as resolved.
Show resolved Hide resolved

// Update parameters from commandline arguments
if let Some(bits) = args.validate {
Expand All @@ -282,14 +277,14 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
params.spv_in = naga::front::spv::Options {
adjust_coordinate_space: !args.keep_coordinate_space,
strict_capabilities: false,
block_ctx_dump_prefix: args.block_ctx_dir.map(std::path::PathBuf::from),
block_ctx_dump_prefix: args.block_ctx_dir.clone().map(std::path::PathBuf::from),
};

params.entry_point = args.entry_point;
if let Some(version) = args.profile {
params.entry_point = args.entry_point.clone();
if let Some(ref version) = args.profile {
params.glsl.version = version.0;
}
if let Some(model) = args.shader_model {
if let Some(ref model) = args.shader_model {
params.hlsl.shader_model = model.0;
}
params.keep_coordinate_space = args.keep_coordinate_space;
Expand All @@ -302,68 +297,26 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
!params.keep_coordinate_space,
);

let (mut module, input_text) = match Path::new(&input_path)
.extension()
.ok_or(CliError("Input filename has no extension"))?
.to_str()
.ok_or(CliError("Input filename not valid unicode"))?
{
"bin" => (bincode::deserialize(&input)?, None),
"spv" => naga::front::spv::parse_u8_slice(&input, &params.spv_in).map(|m| (m, None))?,
"wgsl" => {
let input = String::from_utf8(input)?;
let result = naga::front::wgsl::parse_str(&input);
match result {
Ok(v) => (v, Some(input)),
Err(ref e) => {
e.emit_to_stderr_with_path(&input, input_path);
return Err(CliError("Could not parse WGSL").into());
}
}
}
ext @ ("vert" | "frag" | "comp" | "glsl") => {
let input = String::from_utf8(input)?;
let mut parser = naga::front::glsl::Frontend::default();
if args.bulk_validate {
return bulk_validate(args, &params);
}

(
parser
.parse(
&naga::front::glsl::Options {
stage: match ext {
"vert" => naga::ShaderStage::Vertex,
"frag" => naga::ShaderStage::Fragment,
"comp" => naga::ShaderStage::Compute,
"glsl" => {
let internal_name = input_path.to_string_lossy();
match Path::new(&internal_name[..internal_name.len()-5])
.extension()
.ok_or(CliError("Input filename ending with .glsl has no internal extension"))?
.to_str()
.ok_or(CliError("Input filename not valid unicode"))?
{
"vert" => naga::ShaderStage::Vertex,
"frag" => naga::ShaderStage::Fragment,
"comp" => naga::ShaderStage::Compute,
_ => unreachable!(),
}
},
_ => unreachable!(),
},
defines: Default::default(),
},
&input,
)
.unwrap_or_else(|errors| {
let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str);
emit_glsl_parser_error(errors, filename.unwrap_or("glsl"), &input);
std::process::exit(1);
}),
Some(input),
)
}
_ => return Err(CliError("Unknown input file extension").into()),
let (input_path, input) = if let Some(path) = args.files.first() {
let path = Path::new(path);
(path, fs::read(path)?)
} else if let Some(path) = &args.stdin_file_path {
let mut input = vec![];
std::io::stdin().lock().read_to_end(&mut input)?;
(Path::new(path), input)
} else {
return Err(CliError("Input file path is not specified").into());
};

let Parsed {
mut module,
input_text,
} = parse_input(input_path, input, &params)?;

// Include debugging information if requested.
if args.generate_debug_symbols {
if let Some(ref input_text) = input_text {
Expand All @@ -384,6 +337,8 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
}
}

let output_paths = args.files.get(1..).unwrap_or(&[]);

// Decide which capabilities our output formats can support.
let validation_caps =
output_paths
Expand Down Expand Up @@ -469,6 +424,84 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
Ok(())
}

struct Parsed {
module: naga::Module,
input_text: Option<String>,
}

fn parse_input(
input_path: &Path,
input: Vec<u8>,
params: &Parameters,
) -> Result<Parsed, Box<dyn std::error::Error>> {
let (module, input_text) = match Path::new(&input_path)
.extension()
.ok_or(CliError("Input filename has no extension"))?
.to_str()
.ok_or(CliError("Input filename not valid unicode"))?
{
"bin" => (bincode::deserialize(&input)?, None),
"spv" => naga::front::spv::parse_u8_slice(&input, &params.spv_in).map(|m| (m, None))?,
"wgsl" => {
let input = String::from_utf8(input)?;
let result = naga::front::wgsl::parse_str(&input);
match result {
Ok(v) => (v, Some(input)),
Err(ref e) => {
let message = format!(
"Could not parse WGSL:\n{}",
e.emit_to_string_with_path(&input, input_path)
);
return Err(message.into());
}
}
}
ext @ ("vert" | "frag" | "comp" | "glsl") => {
let input = String::from_utf8(input)?;
let mut parser = naga::front::glsl::Frontend::default();

(
parser
.parse(
&naga::front::glsl::Options {
stage: match ext {
"vert" => naga::ShaderStage::Vertex,
"frag" => naga::ShaderStage::Fragment,
"comp" => naga::ShaderStage::Compute,
"glsl" => {
let internal_name = input_path.to_string_lossy();
match Path::new(&internal_name[..internal_name.len()-5])
.extension()
.ok_or(CliError("Input filename ending with .glsl has no internal extension"))?
.to_str()
.ok_or(CliError("Input filename not valid unicode"))?
{
"vert" => naga::ShaderStage::Vertex,
"frag" => naga::ShaderStage::Fragment,
"comp" => naga::ShaderStage::Compute,
_ => unreachable!(),
}
},
_ => unreachable!(),
},
defines: Default::default(),
},
&input,
)
.unwrap_or_else(|errors| {
let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str);
emit_glsl_parser_error(errors, filename.unwrap_or("glsl"), &input);
std::process::exit(1);
}),
Some(input),
)
}
_ => return Err(CliError("Unknown input file extension").into()),
};

Ok(Parsed { module, input_text })
}

fn write_output(
module: &naga::Module,
info: &Option<naga::valid::ModuleInfo>,
Expand Down Expand Up @@ -629,6 +662,53 @@ fn write_output(
Ok(())
}

fn bulk_validate(args: Args, params: &Parameters) -> Result<(), Box<dyn std::error::Error>> {
let mut invalid = vec![];
for input_path in args.files {
let path = Path::new(&input_path);
let input = fs::read(path)?;

let Parsed { module, input_text } = match parse_input(path, input, params) {
Ok(parsed) => parsed,
Err(error) => {
invalid.push(input_path.clone());
eprintln!("Error validating {}:", input_path);
eprintln!("{error}");
jimblandy marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
};

let mut validator =
naga::valid::Validator::new(params.validation_flags, naga::valid::Capabilities::all());

if let Err(error) = validator.validate(&module) {
invalid.push(input_path.clone());
eprintln!("Error validating {}:", input_path);
if let Some(input) = &input_text {
let filename = path.file_name().and_then(std::ffi::OsStr::to_str);
emit_annotated_error(&error, filename.unwrap_or("input"), input);
}
print_err(&error);
}
}

if !invalid.is_empty() {
use std::fmt::Write;
let mut formatted = String::new();
writeln!(
&mut formatted,
"Validation failed for the following inputs:"
)
.unwrap();
for path in invalid {
writeln!(&mut formatted, " {path}").unwrap();
}
return Err(formatted.into());
}

Ok(())
}

use codespan_reporting::{
diagnostic::{Diagnostic, Label},
files::SimpleFile,
Expand Down
14 changes: 10 additions & 4 deletions naga/xtask/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,16 @@ fn run(args: Args) -> anyhow::Result<()> {
})
}
ValidateSubcommand::Wgsl => {
visit_files(snapshots_base_out, "wgsl/*.wgsl", |path| {
ack_visiting(path);
EasyCommand::new("cargo", |cmd| cmd.args(["run", "-p", "naga-cli", "--"]).arg(path)).success()
})
let mut paths = vec![];
let mut error_status = visit_files(snapshots_base_out, "wgsl/*.wgsl", |path| {
paths.push(path.to_owned());
Ok(())
});
EasyCommand::new("cargo", |cmd| {
cmd.args(["run", "-p", "naga-cli", "--", "--bulk-validate"]).args(paths)
}).success()
.log_if_err_found(&mut error_status);
error_status
jimblandy marked this conversation as resolved.
Show resolved Hide resolved
}
ValidateSubcommand::Hlsl(cmd) => {
let visit_hlsl = |consume_config_item: &mut dyn FnMut(
Expand Down
Loading