diff --git a/.github/workflows/lint.sh b/.github/workflows/lint.sh index 66f2d362d5..bdf5f1d59c 100755 --- a/.github/workflows/lint.sh +++ b/.github/workflows/lint.sh @@ -49,15 +49,15 @@ clippy_no_features examples/shaders/simplest-shader # which could be disastrous because env vars access can't be tracked by # `rustc`, unlike its CLI flags (which are integrated with incremental). if ( - egrep -r '::\s*env|env\s*::' crates/rustc_codegen_spirv/src | + grep -E -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\)$' | + grep -E -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\)$' | # HACK(LegNeato) exclude logging. This mirrors `rustc` (`RUSTC_LOG`) and #`rustdoc` (`RUSTDOC_LOG`). # There is not a risk of this being disastrous as it does not change the build settings. - egrep -v '^crates/rustc_codegen_spirv/src/lib.rs:.*(RUSTGPU_LOG|RUSTGPU_LOG_FORMAT|RUSTGPU_LOG_COLOR).*$' | - egrep -v '^crates/rustc_codegen_spirv/src/lib.rs: use std::env::{self, VarError};$' + grep -E -v '^crates/rustc_codegen_spirv/src/lib.rs:.*(RUSTGPU_LOG|RUSTGPU_LOG_FORMAT|RUSTGPU_LOG_COLOR).*$' | + grep -E -v '^crates/rustc_codegen_spirv/src/lib.rs: use std::env::{self, VarError};$' ); then echo '^^^' diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index fa69dc8e7f..98cf3e12a6 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -23,7 +23,7 @@ use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecora use crate::custom_insts; use either::Either; use rspirv::binary::{Assemble, Consumer}; -use rspirv::dr::{Block, Loader, Module, ModuleHeader, Operand}; +use rspirv::dr::{Block, Function, Loader, Module, ModuleHeader, Operand}; use rspirv::spirv::{Op, StorageClass, Word}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::ErrorGuaranteed; @@ -705,6 +705,9 @@ pub fn link( dce::dce(output); } + // Validate fragment shaders after DCE to catch silent optimization failures + validate_fragment_shader_outputs(sess, output)?; + { let _timer = sess.timer("link_remove_duplicate_debuginfo"); duplicates::remove_duplicate_debuginfo(output); @@ -815,3 +818,157 @@ impl Drop for SpirtDumpGuard<'_> { } } } + +/// Validate that fragment shaders have meaningful output operations +/// This catches the case described in issue #284 where fragment shaders +/// are optimized to have no output, causing silent rendering failures +fn validate_fragment_shader_outputs(sess: &Session, module: &Module) -> Result<()> { + use rspirv::spirv::{ExecutionModel, Op}; + + // Find all fragment entry points + for entry in &module.entry_points { + if entry.class.opcode == Op::EntryPoint { + if let Some(execution_model) = entry.operands.first() { + if execution_model.unwrap_execution_model() == ExecutionModel::Fragment { + let function_id = entry.operands[1].unwrap_id_ref(); + let entry_name = entry.operands[2].unwrap_literal_string(); + + // Check if this fragment shader has meaningful output operations + if !fragment_shader_writes_output(module, function_id) { + let mut err = sess.dcx().struct_err(format!( + "fragment shader `{}` produces no output", + entry_name + )); + + err = err.with_help( + "fragment shaders must write to output parameters to produce visible results" + ).with_note( + "use complete assignment like `*out_frag_color = vec4(r, g, b, a)` instead of partial component assignments" + ).with_note( + "partial component assignments may be optimized away if not all components are written" + ); + + // Look for the function to provide better diagnostics + if let Some(func) = module + .functions + .iter() + .find(|f| f.def_id() == Some(function_id)) + { + if has_partial_output_writes(module, func) { + err = err.with_note( + "detected partial component writes (e.g., `out.x = value`) which were optimized away" + ).with_help( + "write all components at once: `*out_frag_color = vec4(r, g, b, 1.0)`" + ); + } + } + + return Err(err.emit()); + } + } + } + } + } + + Ok(()) +} + +/// Check if a fragment shader function has any meaningful output writes +fn fragment_shader_writes_output(module: &Module, function_id: Word) -> bool { + // Find the function definition + let function = match module + .functions + .iter() + .find(|f| f.def_id() == Some(function_id)) + { + Some(func) => func, + None => return true, // If function not found, assume it's valid + }; + + // Check all instructions in the function for output writes + for block in &function.blocks { + for inst in &block.instructions { + if inst.class.opcode == Op::Store { + if let Some(target_id) = inst.operands.first().and_then(|op| op.id_ref_any()) { + if is_output_storage_variable(module, target_id) { + return true; + } + } + } + + // Check function calls recursively + if inst.class.opcode == Op::FunctionCall { + if let Some(callee_id) = inst.operands.first().and_then(|op| op.id_ref_any()) { + if fragment_shader_writes_output(module, callee_id) { + return true; + } + } + } + } + } + + false +} + +/// Check if a fragment shader has partial output writes that might have been optimized away +fn has_partial_output_writes(module: &Module, function: &Function) -> bool { + use rspirv::spirv::Op; + + // Look for AccessChain operations on output variables + // This suggests the shader was trying to write individual components + for block in &function.blocks { + for inst in &block.instructions { + if matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain) { + if let Some(base_id) = inst.operands.first().and_then(|op| op.id_ref_any()) { + if is_output_storage_variable(module, base_id) { + return true; + } + } + } + } + } + + false +} + +/// Check if a variable ID refers to an Output storage class variable +fn is_output_storage_variable(module: &Module, var_id: Word) -> bool { + use rspirv::spirv::{Op, StorageClass}; + + // Check direct output variables + for inst in &module.types_global_values { + if inst.result_id == Some(var_id) && inst.class.opcode == Op::Variable { + if let Some(storage_class) = inst.operands.first() { + return storage_class.unwrap_storage_class() == StorageClass::Output; + } + } + } + + // Check if this is a pointer derived from an output variable + for inst in &module.types_global_values { + if inst.result_id == Some(var_id) + && matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain) + { + if let Some(base_id) = inst.operands.first().and_then(|op| op.id_ref_any()) { + return is_output_storage_variable(module, base_id); + } + } + } + + // Check in function bodies for derived pointers + for function in &module.functions { + for block in &function.blocks { + for inst in &block.instructions { + if inst.result_id == Some(var_id) + && matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain) + { + if let Some(base_id) = inst.operands.first().and_then(|op| op.id_ref_any()) { + return is_output_storage_variable(module, base_id); + } + } + } + } + } + + false +} diff --git a/tests/compiletests/ui/dis/issue-284-dead-fragment.rs b/tests/compiletests/ui/dis/issue-284-dead-fragment.rs new file mode 100644 index 0000000000..7506d02de0 --- /dev/null +++ b/tests/compiletests/ui/dis/issue-284-dead-fragment.rs @@ -0,0 +1,12 @@ +//@ compile-flags: --crate-type dylib --emit=metadata +// Test for issue #284: A fragment shader that produces no output should fail + +use spirv_std::spirv; + +#[spirv(fragment)] +pub fn main_fs(#[spirv(flat)] in_color: u32, out_frag_color: &mut spirv_std::glam::Vec4) { + // This fragment shader reads input but doesn't write output + // The assignment is optimized away, causing no visible output + let _temp = in_color; + // Note: No assignment to out_frag_color, so this shader produces no output +} diff --git a/tests/compiletests/ui/dis/issue-284-dead-fragment.stderr b/tests/compiletests/ui/dis/issue-284-dead-fragment.stderr new file mode 100644 index 0000000000..7386d31246 --- /dev/null +++ b/tests/compiletests/ui/dis/issue-284-dead-fragment.stderr @@ -0,0 +1,8 @@ +error: fragment shader `main_fs` produces no output + | + = help: fragment shaders must write to output parameters to produce visible results + = note: use complete assignment like `*out_frag_color = vec4(r, g, b, a)` instead of partial component assignments + = note: partial component assignments may be optimized away if not all components are written + +error: aborting due to 1 previous error +