Skip to content

Commit f5eca7c

Browse files
committed
Add rich validation error messages with source spans
Improve spirv-val error messages by looking up source locations from SrcLocDecoration annotations: - Add validation_err.rs module to parse validation errors and look up source spans from the SPIR-V module's custom SrcLocDecoration - Strip SrcLocDecoration and ZombieDecoration before validation (these are invalid on functions) but keep the original module for span lookup - Add --preserve-debug-decorations flag to optionally keep decorations - Add SrcLocDecoration to entry point interface variables so validation errors for conflicting locations show the relevant source spans - Keep decorations in linker output (they're stripped after validation) For location conflict errors, the error message now shows: - The specific variable causing the conflict with its source location - The conflicting variable with its source location - A help message explaining why the conflict occurs (e.g., Mat4 uses 4 locations)
1 parent 5d9c33b commit f5eca7c

6 files changed

Lines changed: 375 additions & 15 deletions

File tree

crates/rustc_codegen_spirv/src/codegen_cx/entry.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::abi::ConvSpirvType;
66
use crate::attr::{AggregatedSpirvAttributes, Entry, Spanned, SpecConstant};
77
use crate::builder::Builder;
88
use crate::builder_spirv::{SpirvFunctionCursor, SpirvValue, SpirvValueExt};
9+
use crate::custom_decorations::{CustomDecoration, SrcLocDecoration};
910
use crate::spirv_type::SpirvType;
1011
use rspirv::dr::Operand;
1112
use rspirv::spirv::{
@@ -987,6 +988,14 @@ impl<'tcx> CodegenCx<'tcx> {
987988
self.emit_global()
988989
.variable(var_ptr_spirv_type, Some(var), storage_class, None);
989990

991+
// Add SrcLocDecoration to the variable for rich error messages.
992+
let src_loc_inst = SrcLocDecoration::from_rustc_span(hir_param.span, &self.builder)
993+
.map(|src_loc| src_loc.encode_to_inst(var));
994+
self.emit_global()
995+
.module_mut()
996+
.annotations
997+
.extend(src_loc_inst);
998+
990999
// Record this `OpVariable` as needing to be added (if applicable),
9911000
// to the *Interface* operands of the `OpEntryPoint` instruction.
9921001
if self.emit_global().version().unwrap() > (1, 3) {

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,10 @@ pub struct CodegenArgs {
313313

314314
pub run_spirv_opt: bool,
315315

316+
/// Preserve internal debug decorations (SrcLocDecoration, ZombieDecoration) in the output.
317+
/// These are normally stripped after validation. Enable for debugging or testing.
318+
pub preserve_debug_decorations: bool,
319+
316320
/// All options pertinent to `rustc_codegen_spirv::linker` specifically.
317321
//
318322
// FIXME(eddyb) should these be handled as `-C linker-args="..."` instead?
@@ -384,6 +388,12 @@ impl CodegenArgs {
384388
"disables running spirv-opt on the final output",
385389
);
386390

391+
opts.optflag(
392+
"",
393+
"preserve-debug-decorations",
394+
"preserve internal debug decorations (SrcLocDecoration, ZombieDecoration) in the output",
395+
);
396+
387397
opts.optflag(
388398
"",
389399
"preserve-bindings",
@@ -652,6 +662,8 @@ impl CodegenArgs {
652662

653663
run_spirv_opt,
654664

665+
preserve_debug_decorations: matches.opt_present("preserve-debug-decorations"),
666+
655667
linker_opts,
656668

657669
// NOTE(eddyb) these are debugging options that used to be env vars

crates/rustc_codegen_spirv/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ mod spirv_type_constraints;
138138
mod symbols;
139139
mod target;
140140
mod target_feature;
141+
mod validation_err;
141142

142143
use builder::Builder;
143144
use codegen_cx::CodegenCx;

crates/rustc_codegen_spirv/src/link.rs

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,27 @@ fn post_link_single_module(
253253
out_filename: &Path,
254254
dump_prefix: Option<&OsStr>,
255255
) {
256-
cg_args.do_disassemble(&module);
257-
let spv_binary = module.assemble();
256+
use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration};
257+
258+
// Strip internal debug decorations (SrcLocDecoration, ZombieDecoration) BEFORE
259+
// disassembly/validation/optimization unless preserve_debug_decorations is set.
260+
// These decorations are only valid on certain targets (variables), but rust-gpu
261+
// also places them on functions for span tracking which is invalid SPIR-V.
262+
// Keep the original module for span lookup in error messages.
263+
let (module_for_output, original_module_for_spans) = if cg_args.preserve_debug_decorations {
264+
// When preserving decorations, use the same module for both
265+
(module.clone(), module)
266+
} else {
267+
let mut stripped = module.clone();
268+
SrcLocDecoration::remove_all(&mut stripped);
269+
ZombieDecoration::remove_all(&mut stripped);
270+
(stripped, module)
271+
};
272+
273+
// Disassemble the stripped module (unless preserving decorations)
274+
cg_args.do_disassemble(&module_for_output);
275+
let spv_binary = module_for_output.assemble();
276+
let original_binary_for_spans = original_module_for_spans.assemble();
258277

259278
if let Some(dir) = &cg_args.dump_post_link {
260279
// FIXME(eddyb) rename `filename` with `file_path` to make this less confusing.
@@ -305,7 +324,7 @@ fn post_link_single_module(
305324
};
306325

307326
if cg_args.run_spirv_val {
308-
do_spirv_val(sess, &spv_binary, out_filename, val_options);
327+
do_spirv_val(sess, &spv_binary, &original_binary_for_spans, out_filename, val_options);
309328
}
310329

311330
{
@@ -393,18 +412,25 @@ fn do_spirv_opt(
393412
fn do_spirv_val(
394413
sess: &Session,
395414
spv_binary: &[u32],
415+
original_binary_for_spans: &[u32],
396416
filename: &Path,
397417
options: spirv_tools::val::ValidatorOptions,
398418
) {
419+
use crate::validation_err::ValidationErrorContext;
399420
use spirv_tools::val::{self, Validator};
400421

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

403424
if let Err(e) = validator.validate(spv_binary, Some(options)) {
404-
let mut err = sess.dcx().struct_err(e.to_string());
405-
err.note("spirv-val failed");
406-
err.note(format!("module `{}`", filename.display()));
407-
err.emit();
425+
// Parse the ORIGINAL binary (with SrcLocDecoration) to look up source spans
426+
// Note: We validate the stripped binary, but look up spans in the original
427+
let module = with_rspirv_loader(|loader| {
428+
rspirv::binary::parse_words(original_binary_for_spans, loader)
429+
})
430+
.ok();
431+
432+
let mut ctx = ValidationErrorContext::new(sess, module.as_ref(), filename);
433+
ctx.emit_error(&e);
408434
}
409435
}
410436

crates/rustc_codegen_spirv/src/linker/mod.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ mod zombies;
1919
use std::borrow::Cow;
2020

2121
use crate::codegen_cx::{ModuleOutputType, SpirvMetadata};
22-
use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration};
22+
// Note: SrcLocDecoration and ZombieDecoration are stripped in link.rs after validation
2323
use crate::custom_insts;
2424
use either::Either;
2525
use rspirv::binary::Assemble;
@@ -768,13 +768,10 @@ pub fn link(
768768
output.header.as_mut().unwrap().bound = simple_passes::compact_ids(output);
769769
};
770770

771-
// FIXME(eddyb) convert these into actual `OpLine`s with a SPIR-T pass,
772-
// but that'd require keeping the modules in SPIR-T form (once lowered),
773-
// and never loading them back into `rspirv` once lifted back to SPIR-V.
774-
SrcLocDecoration::remove_all(output);
775-
776-
// FIXME(eddyb) might make more sense to rewrite these away on SPIR-T.
777-
ZombieDecoration::remove_all(output);
771+
// NOTE: We keep SrcLocDecoration and ZombieDecoration in the module
772+
// so that they can be used for rich error messages during validation.
773+
// These are encoded as OpDecorateString with UserSemantic, which is
774+
// valid SPIR-V and should not cause validation failures.
778775
}
779776

780777
Ok(output)

0 commit comments

Comments
 (0)