diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a5a216ecd8..2e66bdcdea 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -151,6 +151,27 @@ jobs: run: RUSTDOCFLAGS=-Dwarnings cargo doc --no-deps - name: Clippy run: .github/workflows/clippy.sh + # HACK(eddyb) see `docs/src/codegen-args.md` for more context around this, + # but basically we're implementing a custom "lint" to ban `std::env` usage, + # which could be disastrous because env vars access can't be tracked by + # `rustc`, unlike its CLI flags (which are integrated with incremental). + - name: Disallow `std::env` (mis)use from `rustc_codegen_spirv`` + run: | + if ( + egrep -r '::\s*env|env\s*::' crates/rustc_codegen_spirv/src/ | + # HACK(eddyb) exclude the one place in `rustc_codegen_spirv` + # needing access to an env var (only for codegen args `--help`). + egrep -v '^crates/rustc_codegen_spirv/src/codegen_cx/mod.rs: let help_flag_comes_from_spirv_builder_env_var = std::env::var\(spirv_builder_env_var\)$' + ); then + echo '^^^' + echo '!!! Found disallowed `std::env` usage in `rustc_codegen_spirv` !!!' + echo ' ("codegen args" should be used instead of environment variables)' + echo + echo 'For more details on "codegen args", see: docs/src/codegen-args.md' + echo ' (and/or https://github.com/EmbarkStudios/rust-gpu/pull/959)' + echo + exit 1 + fi cargo-deny: runs-on: ubuntu-20.04 diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cd3af957b..a000f66ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,10 +29,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added ⭐ + +- Added two `spirv-builder` environment variables to customize *only* the `rustc` invocations for shader crates and their dependencies: + - `RUSTGPU_RUSTFLAGS="..."` for shader `RUSTFLAGS="..."` + - `RUSTGPU_CODEGEN_ARGS="..."` for shader "codegen args" (i.e. `RUSTFLAGS=-Cllvm-args="..."`) + (check out ["codegen args" docs](docs/src/codegen-args.md), or run with `RUSTGPU_CODEGEN_ARGS=--help` to see the full list of options) + ### Changed 🛠️ - Updated toolchain to `nightly-2022-10-29` -- Applied workspace inheritance to Cargo.toml files +- Applied workspace inheritance to `Cargo.toml` files +- Moved `rustc_codegen_spirv` debugging functionality from environment variables to "codegen args" options/flags (see [the updated docs](docs/src/codegen-args.md) for more details) ### Removed 🔥 @@ -51,7 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ⭐ -- Added check for env var `RUSTGPU_SKIP_TOOLCHAIN_CHECK` to prevent toolchain check +- Added check for environment variable `RUSTGPU_SKIP_TOOLCHAIN_CHECK` to prevent toolchain check ### Changed 🛠️ diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index cd535ad0b7..292a5e26e1 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs @@ -31,8 +31,9 @@ use rustc_target::abi::call::{FnAbi, PassMode}; use rustc_target::abi::{HasDataLayout, TargetDataLayout}; use rustc_target::spec::{HasTargetSpec, Target}; use std::cell::{Cell, RefCell}; +use std::collections::BTreeSet; use std::iter::once; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str::FromStr; @@ -231,8 +232,9 @@ impl<'tcx> CodegenCx<'tcx> { } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, Default, PartialEq, Eq, Clone, Copy)] pub enum SpirvMetadata { + #[default] None, NameVariables, Full, @@ -247,6 +249,8 @@ pub struct CodegenArgs { pub spirv_metadata: SpirvMetadata, + pub run_spirv_val: bool, + // spirv-val flags pub relax_struct_store: bool, pub relax_logical_pointer: bool, @@ -255,8 +259,22 @@ pub struct CodegenArgs { pub scalar_block_layout: bool, pub skip_block_layout: bool, + pub run_spirv_opt: bool, + // spirv-opt flags pub preserve_bindings: bool, + + /// All options pertinent to `rustc_codegen_spirv::linker` specifically. + // + // FIXME(eddyb) should these be handled as `-C linker-args="..."` instead? + pub linker_opts: crate::linker::Options, + + // NOTE(eddyb) these are debugging options that used to be env vars + // (for more information see `docs/src/codegen-args.md`). + pub dump_mir: Option, + pub dump_module_on_panic: Option, + pub dump_pre_link: Option, + pub dump_post_link: Option, } impl CodegenArgs { @@ -267,16 +285,22 @@ impl CodegenArgs { } } + // FIXME(eddyb) `structopt` would come a long way to making this nicer. pub fn parse(args: &[String]) -> Result { use rustc_session::getopts; + + // FIXME(eddyb) figure out what casing ("Foo bar" vs "foo bar") to use + // for the descriptions, `rustc` seems a bit inconsistent itself on this. + let mut opts = getopts::Options::new(); + opts.optflag("h", "help", "Display this message"); opts.optopt( "", "module-output", "single output or multiple output", "[single|multiple]", ); - opts.optflagopt("", "disassemble", "print module to stderr", ""); + opts.optflag("", "disassemble", "print module to stderr"); opts.optopt("", "disassemble-fn", "print function to stderr", "NAME"); opts.optopt( "", @@ -284,25 +308,156 @@ impl CodegenArgs { "print entry point to stderr", "NAME", ); - opts.optflagopt("", "disassemble-globals", "print globals to stderr", ""); + opts.optflag("", "disassemble-globals", "print globals to stderr"); opts.optopt("", "spirv-metadata", "how much metadata to include", ""); - 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.", ""); + // FIXME(eddyb) clean up this `no-` "negation prefix" situation. + opts.optflag( + "", + "no-spirv-val", + "disables running spirv-val on the final output", + ); + + opts.optflag("", "relax-struct-store", "Allow store from one struct type to a different type with compatible layout and members."); + opts.optflag("", "relax-logical-pointer", "Allow allocating an object of a pointer type and returning a pointer value from a function in logical addressing mode"); + opts.optflag("", "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.optflag("", "uniform-buffer-standard-layout", "Enable VK_KHR_uniform_buffer_standard_layout when checking standard uniform buffer layouts."); + opts.optflag("", "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.optflag("", "skip-block-layout", "Skip checking standard uniform/storage buffer layout. Overrides any --relax-block-layout or --scalar-block-layout option."); + + // FIXME(eddyb) clean up this `no-` "negation prefix" situation. + opts.optflag( + "", + "no-spirv-opt", + "disables running spirv-opt on the final output", + ); - opts.optflagopt( + opts.optflag( "", "preserve-bindings", "Preserve unused descriptor bindings. Useful for reflection.", + ); + + // Linker options. + // FIXME(eddyb) should these be handled as `-C linker-args="..."` instead? + { + // FIXME(eddyb) clean up this `no-` "negation prefix" situation. + opts.optflag("", "no-dce", "disables running dead code elimination"); + opts.optflag( + "", + "no-compact-ids", + "disables compaction of SPIR-V IDs at the end of linking", + ); + opts.optflag("", "no-structurize", "disables CFG structurization"); + + // NOTE(eddyb) these are debugging options that used to be env vars + // (for more information see `docs/src/codegen-args.md`). + opts.optopt( + "", + "dump-post-merge", + "dump the merged module immediately after merging, to FILE", + "FILE", + ); + opts.optopt( + "", + "dump-post-split", + "dump modules immediately after multimodule splitting, to files in DIR", + "DIR", + ); + opts.optflag( + "", + "specializer-debug", + "enable debug logging for the specializer", + ); + opts.optopt( + "", + "specializer-dump-instances", + "dump all instances inferred by the specializer, to FILE", + "FILE", + ); + opts.optflag("", "print-all-zombie", "prints all removed zombies"); + opts.optflag( + "", + "print-zombie", + "prints everything removed (even transitively) due to zombies", + ); + } + + // NOTE(eddyb) these are debugging options that used to be env vars + // (for more information see `docs/src/codegen-args.md`). + opts.optopt( + "", + "dump-mir", + "dump every MIR body codegen sees, to files in DIR", + "DIR", + ); + opts.optopt( + "", + "dump-module-on-panic", + "if codegen panics, dump the (partially) emitted module, to FILE", + "FILE", + ); + opts.optopt( + "", + "dump-pre-link", + "dump all input modules to the linker, to files in DIR", + "DIR", + ); + opts.optopt( "", + "dump-post-link", + "dump all output modules from the linker, to files in DIR", + "DIR", ); let matches = opts.parse(args)?; + + let help_flag_positions: BTreeSet<_> = ["h", "help"] + .iter() + .flat_map(|&name| matches.opt_positions(name)) + .collect(); + if !help_flag_positions.is_empty() { + // HACK(eddyb) this tries to be a bit nicer to end-users, when they + // use `spirv-builder` (and so the `RUSTGPU_CODEGEN_ARGS` env var, + // to set codegen args), as mentioning `-Cllvm-args` is suboptimal. + let spirv_builder_env_var = "RUSTGPU_CODEGEN_ARGS"; + let help_flag_comes_from_spirv_builder_env_var = std::env::var(spirv_builder_env_var) + .ok() + .and_then(|args_from_env| { + let args_from_env: Vec<_> = args_from_env.split_whitespace().collect(); + if args_from_env.is_empty() { + return None; + } + + // HACK(eddyb) this may be a bit inefficient but we want to + // make sure that *at least one* of the `-h`/`--help` flags + // came from the `spirv-builder`-supported env var *and* + // that the env var's contents are fully contained in the + // `-C llvm-args` this `rustc` invocation is seeing. + args.windows(args_from_env.len()) + .enumerate() + .filter(|&(_, w)| w == args_from_env) + .map(|(w_start, w)| w_start..w_start + w.len()) + .flat_map(|w_range| help_flag_positions.range(w_range)) + .next() + }) + .is_some(); + let codegen_args_lhs = if help_flag_comes_from_spirv_builder_env_var { + spirv_builder_env_var + } else { + "rustc -Cllvm-args" + }; + println!( + "{}", + opts.usage(&format!( + "Usage: {codegen_args_lhs}=\"...\" with `...` from:" + )) + ); + // HACK(eddyb) this avoids `Cargo` continuing after the message is printed. + std::process::exit(1); + } + let module_output_type = matches.opt_get_default("module-output", ModuleOutputType::Single)?; let disassemble = matches.opt_present("disassemble"); @@ -312,6 +467,9 @@ impl CodegenArgs { let spirv_metadata = matches.opt_str("spirv-metadata"); + // FIXME(eddyb) clean up this `no-` "negation prefix" situation. + let run_spirv_val = !matches.opt_present("no-spirv-val"); + 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"); @@ -319,6 +477,9 @@ impl CodegenArgs { let scalar_block_layout = matches.opt_present("scalar-block-layout"); let skip_block_layout = matches.opt_present("skip-block-layout"); + // FIXME(eddyb) clean up this `no-` "negation prefix" situation. + let run_spirv_opt = !matches.opt_present("no-spirv-opt"); + let preserve_bindings = matches.opt_present("preserve-bindings"); let relax_block_layout = if relax_block_layout { Some(true) } else { None }; @@ -334,6 +495,37 @@ impl CodegenArgs { } }; + let matches_opt_path = |name| matches.opt_str(name).map(PathBuf::from); + let matches_opt_dump_dir_path = |name| { + matches_opt_path(name).map(|path| { + if path.is_file() { + std::fs::remove_file(&path).unwrap(); + } + std::fs::create_dir_all(&path).unwrap(); + path + }) + }; + // FIXME(eddyb) should these be handled as `-C linker-args="..."` instead? + let linker_opts = crate::linker::Options { + // FIXME(eddyb) clean up this `no-` "negation prefix" situation. + dce: !matches.opt_present("no-dce"), + compact_ids: !matches.opt_present("no-compact-ids"), + structurize: !matches.opt_present("no-structurize"), + + // FIXME(eddyb) deduplicate between `CodegenArgs` and `linker::Options`. + emit_multiple_modules: module_output_type == ModuleOutputType::Multiple, + spirv_metadata, + + // NOTE(eddyb) these are debugging options that used to be env vars + // (for more information see `docs/src/codegen-args.md`). + dump_post_merge: matches_opt_path("dump-post-merge"), + dump_post_split: matches_opt_dump_dir_path("dump-post-split"), + specializer_debug: matches.opt_present("specializer-debug"), + specializer_dump_instances: matches_opt_path("specializer-dump-instances"), + print_all_zombie: matches.opt_present("print-all-zombie"), + print_zombie: matches.opt_present("print-zombie"), + }; + Ok(Self { module_output_type, disassemble, @@ -343,6 +535,8 @@ impl CodegenArgs { spirv_metadata, + run_spirv_val, + relax_struct_store, relax_logical_pointer, relax_block_layout, @@ -350,7 +544,18 @@ impl CodegenArgs { scalar_block_layout, skip_block_layout, + run_spirv_opt, + preserve_bindings, + + linker_opts, + + // NOTE(eddyb) these are debugging options that used to be env vars + // (for more information see `docs/src/codegen-args.md`). + dump_mir: matches_opt_dump_dir_path("dump-mir"), + dump_module_on_panic: matches_opt_path("dump-module-on-panic"), + dump_pre_link: matches_opt_dump_dir_path("dump-pre-link"), + dump_post_link: matches_opt_dump_dir_path("dump-post-link"), }) } diff --git a/crates/rustc_codegen_spirv/src/lib.rs b/crates/rustc_codegen_spirv/src/lib.rs index 509a40f16c..ccc0056f1a 100644 --- a/crates/rustc_codegen_spirv/src/lib.rs +++ b/crates/rustc_codegen_spirv/src/lib.rs @@ -107,11 +107,10 @@ use rustc_session::Session; use rustc_span::symbol::{sym, Symbol}; use rustc_target::spec::{Target, TargetTriple}; use std::any::Any; -use std::env; use std::fs::{create_dir_all, File}; use std::io::Cursor; use std::io::Write; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::sync::Arc; fn dump_mir<'tcx>( @@ -410,9 +409,8 @@ impl ExtraBackendMethods for SpirvCodegenBackend { let do_codegen = || { let mono_items = cx.codegen_unit.items_in_deterministic_order(cx.tcx); - if let Some(mut path) = get_env_dump_dir("DUMP_MIR") { - path.push(cgu_name.to_string()); - dump_mir(tcx, &mono_items, &path); + if let Some(dir) = &cx.codegen_args.dump_mir { + dump_mir(tcx, &mono_items, &dir.join(cgu_name.to_string())); } for &(mono_item, (linkage, visibility)) in mono_items.iter() { @@ -438,7 +436,7 @@ impl ExtraBackendMethods for SpirvCodegenBackend { // attributes::sanitize(&cx, SanitizerSet::empty(), entry); } }; - if let Ok(ref path) = env::var("DUMP_MODULE_ON_PANIC") { + if let Some(path) = &cx.codegen_args.dump_module_on_panic { let module_dumper = DumpModuleOnPanic { cx: &cx, path }; with_no_trimmed_paths!(do_codegen()); drop(module_dumper); @@ -470,15 +468,14 @@ impl ExtraBackendMethods for SpirvCodegenBackend { struct DumpModuleOnPanic<'a, 'cx, 'tcx> { cx: &'cx CodegenCx<'tcx>, - path: &'a str, + path: &'a Path, } impl Drop for DumpModuleOnPanic<'_, '_, '_> { fn drop(&mut self) { if std::thread::panicking() { - let path: &Path = self.path.as_ref(); - if path.has_root() { - self.cx.builder.dump_module(path); + if self.path.has_root() { + self.cx.builder.dump_module(self.path); } else { println!("{}", self.cx.builder.dump_module_str()); } @@ -486,19 +483,6 @@ impl Drop for DumpModuleOnPanic<'_, '_, '_> { } } -fn get_env_dump_dir(env_var: &str) -> Option { - if let Some(path) = std::env::var_os(env_var) { - let path = PathBuf::from(path); - if path.is_file() { - std::fs::remove_file(&path).unwrap(); - } - std::fs::create_dir_all(&path).unwrap(); - Some(path) - } else { - None - } -} - /// This is the entrypoint for a hot plugged `rustc_codegen_spirv` #[no_mangle] pub fn __rustc_codegen_backend() -> Box { diff --git a/crates/rustc_codegen_spirv/src/link.rs b/crates/rustc_codegen_spirv/src/link.rs index 1b55e61ac9..2695f9c855 100644 --- a/crates/rustc_codegen_spirv/src/link.rs +++ b/crates/rustc_codegen_spirv/src/link.rs @@ -1,4 +1,4 @@ -use crate::codegen_cx::{CodegenArgs, ModuleOutputType, SpirvMetadata}; +use crate::codegen_cx::{CodegenArgs, SpirvMetadata}; use crate::{linker, SpirvCodegenBackend, SpirvModuleBuffer, SpirvThinBuffer}; use ar::{Archive, GnuBuilder, Header}; use rspirv::binary::Assemble; @@ -17,7 +17,6 @@ use rustc_session::config::{CrateType, DebugInfo, Lto, OptLevel, OutputFilenames use rustc_session::output::{check_file_is_writeable, invalid_output_for_target, out_filename}; use rustc_session::utils::NativeLibKind; use rustc_session::Session; -use std::env; use std::ffi::CString; use std::fs::File; use std::io::{BufWriter, Read}; @@ -202,9 +201,12 @@ fn post_link_single_module( spv_binary: Vec, out_filename: &Path, ) { - if let Some(mut path) = crate::get_env_dump_dir("DUMP_POST_LINK") { - path.push(out_filename.file_name().unwrap()); - std::fs::write(path, spirv_tools::binary::from_binary(&spv_binary)).unwrap(); + if let Some(dir) = &cg_args.dump_post_link { + std::fs::write( + dir.join(out_filename.file_name().unwrap()), + spirv_tools::binary::from_binary(&spv_binary), + ) + .unwrap(); } let val_options = spirv_tools::val::ValidatorOptions { @@ -227,7 +229,7 @@ fn post_link_single_module( let spv_binary = if sess.opts.optimize != OptLevel::No || (sess.opts.debuginfo == DebugInfo::None && cg_args.spirv_metadata == SpirvMetadata::None) { - if env::var("NO_SPIRV_OPT").is_err() { + if cg_args.run_spirv_opt { let _timer = sess.timer("link_spirv_opt"); do_spirv_opt(sess, cg_args, spv_binary, out_filename, opt_options) } else { @@ -237,7 +239,7 @@ fn post_link_single_module( (optlevel, true) => format!("optlevel={:?}, debuginfo=None", optlevel), }; sess.warn(format!( - "spirv-opt should have ran ({}) but was disabled by NO_SPIRV_OPT", + "`spirv-opt` should have ran ({}) but was disabled by `--no-spirv-opt`", reason )); spv_binary @@ -246,7 +248,7 @@ fn post_link_single_module( spv_binary }; - if env::var("NO_SPIRV_VAL").is_err() { + if cg_args.run_spirv_val { do_spirv_val(sess, &spv_binary, out_filename, val_options); } @@ -548,7 +550,7 @@ fn do_link( } } - if let Some(dir) = crate::get_env_dump_dir("DUMP_PRE_LINK") { + if let Some(dir) = &cg_args.dump_pre_link { for (num, module) in modules.iter().enumerate() { std::fs::write( dir.join(format!("mod_{}.spv", num)), @@ -560,15 +562,7 @@ fn do_link( drop(load_modules_timer); // Do the link... - let options = linker::Options { - dce: env::var("NO_DCE").is_err(), - compact_ids: env::var("NO_COMPACT_IDS").is_err(), - structurize: env::var("NO_STRUCTURIZE").is_err(), - emit_multiple_modules: cg_args.module_output_type == ModuleOutputType::Multiple, - spirv_metadata: cg_args.spirv_metadata, - }; - - let link_result = linker::link(sess, modules, &options); + let link_result = linker::link(sess, modules, &cg_args.linker_opts); match link_result { Ok(v) => v, diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 667884d7a3..25876347d6 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -25,15 +25,27 @@ use rspirv::spirv::{Op, StorageClass, Word}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::ErrorGuaranteed; use rustc_session::Session; +use std::path::PathBuf; pub type Result = std::result::Result; +#[derive(Default)] pub struct Options { pub compact_ids: bool, pub dce: bool, pub structurize: bool, + pub emit_multiple_modules: bool, pub spirv_metadata: SpirvMetadata, + + // NOTE(eddyb) these are debugging options that used to be env vars + // (for more information see `docs/src/codegen-args.md`). + pub dump_post_merge: Option, + pub dump_post_split: Option, + pub specializer_debug: bool, + pub specializer_dump_instances: Option, + pub print_all_zombie: bool, + pub print_zombie: bool, } pub enum LinkResult { @@ -147,7 +159,7 @@ pub fn link(sess: &Session, mut inputs: Vec, opts: &Options) -> Result, opts: &Options) -> Result, opts: &Options) -> Result, opts: &Options) -> Result Box::new(m.values_mut()), }; for (i, output) in output_module_iter.enumerate() { - if let Some(mut path) = crate::get_env_dump_dir("DUMP_POST_SPLIT") { - path.push(format!("mod_{}.spv", i)); - std::fs::write(path, spirv_tools::binary::from_binary(&output.assemble())).unwrap(); + if let Some(dir) = &opts.dump_post_split { + std::fs::write( + dir.join(format!("mod_{}.spv", i)), + spirv_tools::binary::from_binary(&output.assemble()), + ) + .unwrap(); } // Run DCE again, even if emit_multiple_modules==false - the first DCE ran before // structurization and mem2reg (for perf reasons), and mem2reg may remove references to diff --git a/crates/rustc_codegen_spirv/src/linker/specializer.rs b/crates/rustc_codegen_spirv/src/linker/specializer.rs index 6895d618a3..2d8ac5424a 100644 --- a/crates/rustc_codegen_spirv/src/linker/specializer.rs +++ b/crates/rustc_codegen_spirv/src/linker/specializer.rs @@ -106,10 +106,14 @@ impl bool> Specialization for SimpleSpecialization { } } -pub fn specialize(module: Module, specialization: impl Specialization) -> Module { +pub fn specialize( + opts: &super::Options, + module: Module, + specialization: impl Specialization, +) -> Module { // FIXME(eddyb) use `log`/`tracing` instead. - let debug = std::env::var("SPECIALIZER_DEBUG").is_ok(); - let dump_instances = std::env::var("SPECIALIZER_DUMP_INSTANCES").ok(); + let debug = opts.specializer_debug; + let dump_instances = &opts.specializer_dump_instances; let mut debug_names = FxHashMap::default(); if debug || dump_instances.is_some() { diff --git a/crates/rustc_codegen_spirv/src/linker/test.rs b/crates/rustc_codegen_spirv/src/linker/test.rs index 6d1196fd80..239f2991b8 100644 --- a/crates/rustc_codegen_spirv/src/linker/test.rs +++ b/crates/rustc_codegen_spirv/src/linker/test.rs @@ -1,5 +1,4 @@ use super::{link, LinkResult, Options}; -use crate::codegen_cx::SpirvMetadata; use pipe::pipe; use rspirv::dr::{Loader, Module}; use rustc_errors::registry::Registry; @@ -124,10 +123,7 @@ fn assemble_and_link(binaries: &[&[u8]]) -> Result { modules, &Options { compact_ids: true, - dce: false, - structurize: false, - emit_multiple_modules: false, - spirv_metadata: SpirvMetadata::None, + ..Default::default() }, ); assert_eq!(sess.has_errors(), res.as_ref().err().copied()); diff --git a/crates/rustc_codegen_spirv/src/linker/zombies.rs b/crates/rustc_codegen_spirv/src/linker/zombies.rs index b888ee47b0..47cc66bd5b 100644 --- a/crates/rustc_codegen_spirv/src/linker/zombies.rs +++ b/crates/rustc_codegen_spirv/src/linker/zombies.rs @@ -7,7 +7,6 @@ use rspirv::spirv::Word; use rustc_data_structures::fx::FxHashMap; use rustc_session::Session; use rustc_span::{Span, DUMMY_SP}; -use std::env; use std::iter::once; #[derive(Clone)] @@ -134,7 +133,11 @@ fn report_error_zombies( result } -pub fn remove_zombies(sess: &Session, module: &mut Module) -> super::Result<()> { +pub fn remove_zombies( + sess: &Session, + opts: &super::Options, + module: &mut Module, +) -> super::Result<()> { let zombies_owned = ZombieDecoration::decode_all(module) .map(|(id, zombie)| { let ZombieDecoration { reason, span } = zombie.deserialize(); @@ -154,7 +157,8 @@ pub fn remove_zombies(sess: &Session, module: &mut Module) -> super::Result<()> let result = report_error_zombies(sess, module, &zombies); - if env::var("PRINT_ALL_ZOMBIE").is_ok() { + // FIXME(eddyb) use `log`/`tracing` instead. + if opts.print_all_zombie { for (&zomb, reason) in &zombies { let orig = if zombies_owned.iter().any(|&(z, _)| z == zomb) { "original" @@ -165,7 +169,7 @@ pub fn remove_zombies(sess: &Session, module: &mut Module) -> super::Result<()> } } - if env::var("PRINT_ZOMBIE").is_ok() { + if opts.print_zombie { let names = get_names(module); for f in &module.functions { if let Some(reason) = is_zombie(f.def.as_ref().unwrap(), &zombies) { diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index f86f81404d..5ad1259945 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -428,36 +428,51 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { "-Zcrate-attr=register_tool(rust_gpu)".to_string(), ]; + // Wrapper for `env::var` that appropriately informs Cargo of the dependency. + let tracked_env_var_get = |name| { + if let MetadataPrintout::Full | MetadataPrintout::DependencyOnly = builder.print_metadata { + println!("cargo:rerun-if-env-changed={name}"); + } + env::var(name) + }; + let mut llvm_args = vec![]; if builder.multimodule { - llvm_args.push("--module-output=multiple"); + llvm_args.push("--module-output=multiple".to_string()); } match builder.spirv_metadata { SpirvMetadata::None => (), - SpirvMetadata::NameVariables => llvm_args.push("--spirv-metadata=name-variables"), - SpirvMetadata::Full => llvm_args.push("--spirv-metadata=full"), + SpirvMetadata::NameVariables => { + llvm_args.push("--spirv-metadata=name-variables".to_string()); + } + SpirvMetadata::Full => llvm_args.push("--spirv-metadata=full".to_string()), } if builder.relax_struct_store { - llvm_args.push("--relax-struct-store"); + llvm_args.push("--relax-struct-store".to_string()); } if builder.relax_logical_pointer { - llvm_args.push("--relax-logical-pointer"); + llvm_args.push("--relax-logical-pointer".to_string()); } if builder.relax_block_layout { - llvm_args.push("--relax-block-layout"); + llvm_args.push("--relax-block-layout".to_string()); } if builder.uniform_buffer_standard_layout { - llvm_args.push("--uniform-buffer-standard-layout"); + llvm_args.push("--uniform-buffer-standard-layout".to_string()); } if builder.scalar_block_layout { - llvm_args.push("--scalar-block-layout"); + llvm_args.push("--scalar-block-layout".to_string()); } if builder.skip_block_layout { - llvm_args.push("--skip-block-layout"); + llvm_args.push("--skip-block-layout".to_string()); } if builder.preserve_bindings { - llvm_args.push("--preserve-bindings"); + llvm_args.push("--preserve-bindings".to_string()); + } + + if let Ok(extra_codegen_args) = tracked_env_var_get("RUSTGPU_CODEGEN_ARGS") { + llvm_args.extend(extra_codegen_args.split_whitespace().map(|s| s.to_string())); } + let llvm_args = join_checking_for_separators(llvm_args, " "); if !llvm_args.is_empty() { rustflags.push(["-Cllvm-args=", &llvm_args].concat()); @@ -475,6 +490,10 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { rustflags.push("-Dwarnings".to_string()); } + if let Ok(extra_rustflags) = tracked_env_var_get("RUSTGPU_RUSTFLAGS") { + rustflags.extend(extra_rustflags.split_whitespace().map(|s| s.to_string())); + } + let mut cargo = Command::new("cargo"); cargo.args([ "build", diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 835c5b8817..7f2ec46655 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -4,9 +4,9 @@ - [Contributing to Rust-GPU]() - [Building](./building-rust-gpu.md) - [Testing](./testing.md) - - [Environment variables Rust-GPU reads](./compiler-env-vars.md) + - ["Codegen args" (flags/options) supported by the Rust-GPU codegen backend](./codegen-args.md) - [Minimizing bugs in SPIR-V](./spirv-minimization.md) - - [Publishing rust-gpu on crates.io](./publishing-rust-gpu.md) + - [Publishing Rust-GPU on crates.io](./publishing-rust-gpu.md) - [Platform Support](./platform-support.md) - [Writing Shader Crates](./writing-shader-crates.md) - [Features]() diff --git a/docs/src/codegen-args.md b/docs/src/codegen-args.md new file mode 100644 index 0000000000..076caa1891 --- /dev/null +++ b/docs/src/codegen-args.md @@ -0,0 +1,140 @@ +# "Codegen args" (flags/options) supported by the Rust-GPU codegen backend + +Please keep in mind that many of these flags/options are for internal development, and may break output +unexpectedly and generally muck things up. Please only use these if you know what you're doing. + +Help is also appreciated keeping this document up to date, "codegen args" flags/options may be +added/removed on an ad-hoc basis without much thought, as they're internal development tools, not a +public API - this documentation is only here because these flags/options may be helpful diagnosing +problems for others. + +It's recommended that "codegen args" options that take paths to files or directories are set to full +paths, as the working directory of the compiler might be something wonky and unexpected, and it's +easier to set the full path. + +--- + +## How to set "codegen args" flags/options + +The most convenient method is relying on `spirv-builder` reading the `RUSTGPU_CODEGEN_ARGS` environment variable, e.g.: + +```console +$ RUSTGPU_CODEGEN_ARGS="--no-spirv-val --dump-post-link=$PWD/postlink" cargo run -p example-runner-wgpu +... + Finished release [optimized] target(s) in 15.15s + +$ file postlink/* +postlink/module: Khronos SPIR-V binary, little-endian, version 0x010300, generator 0x1b0000 +``` + +Notable, `RUSTGPU_CODEGEN_ARGS="--help"` can be used to see an "usage" message (which lists all the flags/options, including onesn not listed in this document), via e.g. running a Cargo build that relies on `spirv-builder`. + +However, it's only a convenient alias for for `RUSTGPU_RUSTFLAGS=-Cllvm-args="..."` (without having to expose the fact that LLVM's name is still attached to `rustc`'s interface for this functionality), and if in direct control of `rustc`, you can still pass such "codegen args" flags/options wrapped in `-C llvm-args="..."`. + +--- + +## Historical note about past "environment variables" + +Many of these flags/options were at one point, individual environment variable (e.g. the `--dump-pre-link` option used to be the environment variable `DUMP_PRE_LINK`). + +However, that approach is prone to various failure modes, because the environment variables would not get registered as a "dependency" (without extra work that never happened), and the idea of "codegen args" fits better with existing practices (e.g. `rustc -C llvm-args="..."` for the LLVM codegen backend of `rustc`). + +For more context see also [PR #959](https://github.com/EmbarkStudios/rust-gpu/pull/959), which made the transition to this system. + +## Where are all the rest of the flags/options documented? + +If you do run a build with `RUSTGPU_CODEGEN_ARGS="--help"` (or `-C llvm-args="--help"`), you will notice more flags/options than are listed in this documented. + +This is a historical artifact: as mentioned above, these used to be environment variables, and this document only described those, without talking about the older "codegen args" at all. + +While most of those flags are usually only exposed through higher-level `spirv-builder` APIs, it would be nice to have all of them documented in one place (eventually?). + +--- + +## Debugging "codegen args" flags/options + +As mentioned above, these form the bulk of "codegen args", but keep in mind the list is not exhaustive and you will want to cehck the full list with e.g. `RUSTGPU_CODEGEN_ARGS="--help"`. + +### `--dump-mir DIR` + +Dumps the MIR of every function rust-gpu encounters, to files in `DIR`. Yes, rustc does have options to do +this by default, but I always forget the syntax, and plumbing through the option to `spirv-builder` +is annoying, so this is handy to just hack an output. + +_**FIXME(@eddyb)** this may be irrelevant now given `RUSTGPU_RUSTFLAGS`_ + +### `--dump-module-on-panic FILE` + +If codegen panics, then write the (partially) emitted module to `FILE`. Note that this only exists +for codegen, if the linker panics, this option does nothing, sadly. + +### `--dump-pre-link DIR` + +Dumps all input modules to the linker, to files in `DIR`, before the linker touches them at all. + +### `--dump-post-merge FILE` + +Dumps the merged module, to `FILE`, immediately after merging, but before the linker has done anything else +(including, well, linking the methods - `LinkageAttributes` will still exist, etc.). This is very +similar to `--dump-pre-link`, except it outputs only a single file, which might make grepping through +for stuff easier. + +### `--dump-post-split DIR` + +Dumps the modules, to files in `DIR`, immediately after multimodule splitting, but before final cleanup passes (e.g. +DCE to remove the other entry points). + +### `--dump-post-link DIR` + +Takes: path to directory + +Dumps all output modules from the linker, to files in `DIR`. This may be multiple files due to the multimodule/module +splitting option, hence it takes a directory instead of a file path. This is the final output +binary before `spirv-opt` is executed, so it may be useful to output this to check if an issue is in +Rust-GPU, or in `spirv-opt`. + +### `--specializer-debug` + +Enables debug logging for the specializer. + +_**FIXME(@eddyb)** use `log`/`tracing` instead for this purpose_ + +### `--specializer-dump-instances FILE` + +Dumps to `FILE` all instances inferred by the specializer. + +### `--print-zombie` + +Prints to rustc stdout which functions were removed due to being zombies, and why. + +_**FIXME(@eddyb)** use `log`/`tracing` instead for this purpose_ + +### `--print-all-zombie` + +Prints to rustc stdout *everything* that was removed due to being zombies, why, and if it was an +original zombie or if it was infected. (prints a lot!) + +_**FIXME(@eddyb)** use `log`/`tracing` instead for this purpose_ + +### `--no-spirv-val` + +Disables running `spirv-val` on the final output. Spooky scary option, can cause invalid modules! + +### `--no-spirv-opt` + +Forcibly disables running `spirv-opt` on the final output, even if optimizations are enabled. + +### `--no-dce` + +Disables running dead code elimination. Can and probably will generate invalid modules or crash the +linker, hasn't been tested for a while. + +### `--no-compact-ids` + +Disables compaction of SPIR-V IDs at the end of linking. Causes absolutely ginormous IDs to be +emitted. Useful if you're println debugging IDs in the linker (although spirv-opt will compact them +anyway, be careful). + +### `--no-structurize` + +Disables CFG structurization. Probably results in invalid modules. diff --git a/docs/src/compiler-env-vars.md b/docs/src/compiler-env-vars.md deleted file mode 100644 index 771de4b606..0000000000 --- a/docs/src/compiler-env-vars.md +++ /dev/null @@ -1,117 +0,0 @@ -# Debug environment variables that the Rust-GPU compiler reads - -Please keep in mind that all of these variables are for internal development, and may break output -unexpectedly and generally muck things up. Please only use these if you know what you're doing. - -Help is also appreciated keeping this document up to date, environment variables may be -added/removed on an ad-hoc basis without much thought, as they're internal development tools, not a -public API - this documentation is only here because these variables may be helpful diagnosing -problems for others. - -It's recommended that environment variables that take paths to files or directories are set to full -paths, as the working directory of the compiler might be something wonky and unexpected, and it's -easier to set the full path. - -## DUMP_MIR - -Takes: path to file - -Dumps the MIR of every function rust-gpu encounters to a file. Yes, rustc does have options to do -this by default, but I always forget the syntax, and plumbing through the option to spirv-builder -is annoying, so this is handy to just hack an output. - -## DUMP_MODULE_ON_PANIC - -Takes: path to file - -If codegen panics, then write the (partially) emitted module to a file. Note that this only exists -for codegen, if the linker panics, this option does nothing, sadly. - -## DUMP_PRE_LINK - -Takes: path to directory - -Dumps all input modules to the linker, before the linker touches them at all. - -## DUMP_POST_MERGE - -Takes: path to file - -Dumps the merged module immediately after merging, but before the linker has done anything else -(including, well, linking the methods - LinkageAttributes will still exist, etc.). This is very -similar to DUMP_PRE_LINK, except it outputs only a single file, which might make grepping through -for stuff easier. - -## DUMP_POST_SPLIT - -Takes: path to directory - -Dumps the modules immediately after multimodule splitting, but before final cleanup passes (e.g. -DCE to remove the other entry points). - -## DUMP_POST_LINK - -Takes: path to directory - -Dumps all output modules from the linker. This may be multiple files due to the multimodule/module -splitting option, hence it takes a directory instead of a file path. This is the final output -binary before spirv-opt is executed, so it may be useful to output this to check if an issue is in -rust-gpu, or in spirv-opt. - -## SPECIALIZER_DEBUG - -Takes: presence or absence (e.g. set to `1`) - -Prints to rustc stdout some debug information about the specializer. - -## SPECIALIZER_DUMP_INSTANCES - -Takes: path to file - -Prints to file some... stuff, idk, ask eddyb (useful for debugging specializer) - -## PRINT_ZOMBIE - -Takes: presence or absence (e.g. set to `1`) - -Prints to rustc stdout which functions were removed due to being zombies, and why. - -## PRINT_ALL_ZOMBIE - -Takes: presence or absence (e.g. set to `1`) - -Prints to rustc stdout *everything* that was removed due to being zombies, why, and if it was an -original zombie or if it was infected. (prints a lot!) - -## NO_SPIRV_VAL - -Takes: presence or absence (e.g. set to `1`) - -Disables running spirv-val on the final output. Spooky scary option, can cause invalid modules! - -## NO_SPIRV_OPT - -Takes: presence or absence (e.g. set to `1`) - -Forcibly disables running spirv-opt on the final output, even if optimizations are enabled. - -## NO_DCE - -Takes: presence or absence (e.g. set to `1`) - -Disables running dead code elimination. Can and probably will generate invalid modules or crash the -linker, hasn't been tested for a while. - -## NO_COMPACT_IDS - -Takes: presence or absence (e.g. set to `1`) - -Disables compaction of SPIR-V IDs at the end of linking. Causes absolutely ginormous IDs to be -emitted. Useful if you're println debugging IDs in the linker (although spirv-opt will compact them -anyway, be careful). - -## NO_STRUCTURIZE - -Takes: presence or absence (e.g. set to `1`) - -Disables structurization. Probably results in invalid modules.