From d26716b881eaf8cc1cfd7e304552e876f591a993 Mon Sep 17 00:00:00 2001 From: Ashley Hauck <953151+khyperia@users.noreply.github.com> Date: Tue, 14 Dec 2021 09:04:50 +0100 Subject: [PATCH] Add error for insts not allowed outside fragment (#821) * Add error for insts not allowed outside fragment * make get_name return entry point names too --- crates/rustc_codegen_spirv/src/linker/mod.rs | 22 +++- .../src/linker/simple_passes.rs | 110 +++++++++++++++++- tests/ui/image/implicit_not_in_fragment.rs | 23 ++++ .../ui/image/implicit_not_in_fragment.stderr | 18 +++ .../consts/nested-ref-in-composite.stderr | 4 +- .../core/ptr/allocate_const_scalar.stderr | 2 +- 6 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 tests/ui/image/implicit_not_in_fragment.rs create mode 100644 tests/ui/image/implicit_not_in_fragment.stderr diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index e353977079..0b656e80a8 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -81,7 +81,17 @@ fn apply_rewrite_rules(rewrite_rules: &FxHashMap, blocks: &mut [Bloc } fn get_names(module: &Module) -> FxHashMap { - module + let entry_names = module + .entry_points + .iter() + .filter(|i| i.class.opcode == Op::EntryPoint) + .map(|i| { + ( + i.operands[1].unwrap_id_ref(), + i.operands[2].unwrap_literal_string(), + ) + }); + let debug_names = module .debug_names .iter() .filter(|i| i.class.opcode == Op::Name) @@ -90,8 +100,9 @@ fn get_names(module: &Module) -> FxHashMap { i.operands[0].unwrap_id_ref(), i.operands[1].unwrap_literal_string(), ) - }) - .collect() + }); + // items later on take priority + entry_names.chain(debug_names).collect() } fn get_name<'a>(names: &FxHashMap, id: Word) -> Cow<'a, str> { @@ -157,6 +168,11 @@ pub fn link(sess: &Session, mut inputs: Vec, opts: &Options) -> Result Result<()> { + let mut visited = vec![false; module.functions.len()]; + let mut stack = Vec::new(); + let mut names = None; + let func_id_to_idx: FxHashMap = module + .functions + .iter() + .enumerate() + .map(|(index, func)| (func.def_id().unwrap(), index)) + .collect(); + let entries = module + .entry_points + .iter() + .filter(|i| i.operands[0].unwrap_execution_model() != ExecutionModel::Fragment) + .map(|i| func_id_to_idx[&i.operands[1].unwrap_id_ref()]); + let mut okay = true; + for entry in entries { + okay &= visit( + sess, + module, + &mut visited, + &mut stack, + &mut names, + entry, + &func_id_to_idx, + ); + } + return if okay { Ok(()) } else { Err(ErrorReported) }; + + // returns false if error + fn visit<'m>( + sess: &Session, + module: &'m Module, + visited: &mut Vec, + stack: &mut Vec, + names: &mut Option>, + index: usize, + func_id_to_idx: &FxHashMap, + ) -> bool { + if visited[index] { + return true; + } + visited[index] = true; + stack.push(module.functions[index].def_id().unwrap()); + let mut okay = true; + for inst in module.functions[index].all_inst_iter() { + if inst.class.opcode == Op::FunctionCall { + let called_func = func_id_to_idx[&inst.operands[0].unwrap_id_ref()]; + okay &= visit( + sess, + module, + visited, + stack, + names, + called_func, + func_id_to_idx, + ); + } + if matches!( + inst.class.opcode, + Op::ImageSampleImplicitLod + | Op::ImageSampleDrefImplicitLod + | Op::ImageSampleProjImplicitLod + | Op::ImageSampleProjDrefImplicitLod + | Op::ImageQueryLod + | Op::ImageSparseSampleImplicitLod + | Op::ImageSparseSampleDrefImplicitLod + | Op::DPdx + | Op::DPdy + | Op::Fwidth + | Op::DPdxFine + | Op::DPdyFine + | Op::FwidthFine + | Op::DPdxCoarse + | Op::DPdyCoarse + | Op::FwidthCoarse + | Op::Kill + ) { + // These instructions are (usually) in system functions - if we get an error, allow + // the system function to be visited again from elsewhere to emit another error + // from another callsite. + visited[index] = false; + + let names = names.get_or_insert_with(|| get_names(module)); + let stack = stack.iter().rev().map(|&s| get_name(names, s).into_owned()); + let note = once("Stack:".to_string()) + .chain(stack) + .collect::>() + .join("\n"); + sess.struct_err(&format!( + "{} cannot be used outside a fragment shader", + inst.class.opname + )) + .note(¬e) + .emit(); + okay = false; + } + } + stack.pop(); + okay + } +} diff --git a/tests/ui/image/implicit_not_in_fragment.rs b/tests/ui/image/implicit_not_in_fragment.rs new file mode 100644 index 0000000000..b0030957ff --- /dev/null +++ b/tests/ui/image/implicit_not_in_fragment.rs @@ -0,0 +1,23 @@ +// build-fail + +use spirv_std::{arch, Image, Sampler}; + +fn deeper_stack(image2d: &Image!(2D, type=f32, sampled), sampler: &Sampler) -> glam::Vec4 { + let v2 = glam::Vec2::new(0.0, 1.0); + image2d.sample(*sampler, v2) +} +fn deep_stack(image2d: &Image!(2D, type=f32, sampled), sampler: &Sampler) -> glam::Vec4 { + deeper_stack(image2d, sampler) +} + +#[spirv(vertex)] +pub fn main( + #[spirv(descriptor_set = 0, binding = 0)] image2d: &Image!(2D, type=f32, sampled), + #[spirv(descriptor_set = 0, binding = 1)] sampler: &Sampler, + output: &mut glam::Vec4, +) { + let v2 = glam::Vec2::new(0.0, 1.0); + let r0 = deep_stack(image2d, sampler); + let r1: glam::Vec4 = image2d.sample(*sampler, v2); + *output = r0 + r1; +} diff --git a/tests/ui/image/implicit_not_in_fragment.stderr b/tests/ui/image/implicit_not_in_fragment.stderr new file mode 100644 index 0000000000..5b346cf309 --- /dev/null +++ b/tests/ui/image/implicit_not_in_fragment.stderr @@ -0,0 +1,18 @@ +error: ImageSampleImplicitLod cannot be used outside a fragment shader + | + = note: Stack: + >::sample:: + implicit_not_in_fragment::deeper_stack + implicit_not_in_fragment::deep_stack + implicit_not_in_fragment::main + main + +error: ImageSampleImplicitLod cannot be used outside a fragment shader + | + = note: Stack: + >::sample:: + implicit_not_in_fragment::main + main + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lang/consts/nested-ref-in-composite.stderr b/tests/ui/lang/consts/nested-ref-in-composite.stderr index 4194fdffad..670d530d64 100644 --- a/tests/ui/lang/consts/nested-ref-in-composite.stderr +++ b/tests/ui/lang/consts/nested-ref-in-composite.stderr @@ -6,7 +6,7 @@ error: constant arrays/structs cannot contain pointers to other constants | = note: Stack: nested_ref_in_composite::main_pair - Unnamed function ID %24 + main_pair error: constant arrays/structs cannot contain pointers to other constants --> $DIR/nested-ref-in-composite.rs:27:19 @@ -16,7 +16,7 @@ error: constant arrays/structs cannot contain pointers to other constants | = note: Stack: nested_ref_in_composite::main_array3 - Unnamed function ID %33 + main_array3 error: error:0:0 - No OpEntryPoint instruction was found. This is only allowed if the Linkage capability is being used. | diff --git a/tests/ui/lang/core/ptr/allocate_const_scalar.stderr b/tests/ui/lang/core/ptr/allocate_const_scalar.stderr index eaced17943..865a13dd32 100644 --- a/tests/ui/lang/core/ptr/allocate_const_scalar.stderr +++ b/tests/ui/lang/core/ptr/allocate_const_scalar.stderr @@ -2,7 +2,7 @@ error: pointer has non-null integer address | = note: Stack: allocate_const_scalar::main - Unnamed function ID %4 + main error: error:0:0 - No OpEntryPoint instruction was found. This is only allowed if the Linkage capability is being used. |