Skip to content

Commit c9cf816

Browse files
committed
Fix fragment shader silent optimization (Issue #284)
Addresses issue #284 where fragment shaders were optimized to have no output operations, causing silent rendering failures. The real problem is not DCE eliminating entry points (they are always rooted), but rather fragment shaders being optimized to have no meaningful output writes to StorageClass::Output variables. Implementation: - Add validation after DCE to detect fragment shaders with no output - Provide helpful error messages with examples - Detect partial component writes that may have been optimized away - Recursively check function calls for output operations Test case: - issue-284-dead-fragment.rs: Fragment shader with no output (fails with clear error) The validation catches cases where developers write partial component assignments that get optimized away, providing clear guidance on how to fix the issue.
1 parent e210ab4 commit c9cf816

File tree

3 files changed

+178
-1
lines changed

3 files changed

+178
-1
lines changed

crates/rustc_codegen_spirv/src/linker/mod.rs

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::custom_decorations::{CustomDecoration, SrcLocDecoration, ZombieDecora
2323
use crate::custom_insts;
2424
use either::Either;
2525
use rspirv::binary::{Assemble, Consumer};
26-
use rspirv::dr::{Block, Loader, Module, ModuleHeader, Operand};
26+
use rspirv::dr::{Block, Function, Loader, Module, ModuleHeader, Operand};
2727
use rspirv::spirv::{Op, StorageClass, Word};
2828
use rustc_data_structures::fx::FxHashMap;
2929
use rustc_errors::ErrorGuaranteed;
@@ -710,6 +710,9 @@ pub fn link(
710710
dce::dce(output);
711711
}
712712

713+
// Validate fragment shaders after DCE to catch silent optimization failures
714+
validate_fragment_shader_outputs(sess, output)?;
715+
713716
{
714717
let _timer = sess.timer("link_remove_duplicate_debuginfo");
715718
duplicates::remove_duplicate_debuginfo(output);
@@ -820,3 +823,157 @@ impl Drop for SpirtDumpGuard<'_> {
820823
}
821824
}
822825
}
826+
827+
/// Validate that fragment shaders have meaningful output operations
828+
/// This catches the case described in issue #284 where fragment shaders
829+
/// are optimized to have no output, causing silent rendering failures
830+
fn validate_fragment_shader_outputs(sess: &Session, module: &Module) -> Result<()> {
831+
use rspirv::spirv::{ExecutionModel, Op};
832+
833+
// Find all fragment entry points
834+
for entry in &module.entry_points {
835+
if entry.class.opcode == Op::EntryPoint {
836+
if let Some(execution_model) = entry.operands.first() {
837+
if execution_model.unwrap_execution_model() == ExecutionModel::Fragment {
838+
let function_id = entry.operands[1].unwrap_id_ref();
839+
let entry_name = entry.operands[2].unwrap_literal_string();
840+
841+
// Check if this fragment shader has meaningful output operations
842+
if !fragment_shader_writes_output(module, function_id) {
843+
let mut err = sess.dcx().struct_err(format!(
844+
"fragment shader `{}` produces no output",
845+
entry_name
846+
));
847+
848+
err = err.with_help(
849+
"fragment shaders must write to output parameters to produce visible results"
850+
).with_note(
851+
"use complete assignment like `*out_frag_color = vec4(r, g, b, a)` instead of partial component assignments"
852+
).with_note(
853+
"partial component assignments may be optimized away if not all components are written"
854+
);
855+
856+
// Look for the function to provide better diagnostics
857+
if let Some(func) = module
858+
.functions
859+
.iter()
860+
.find(|f| f.def_id() == Some(function_id))
861+
{
862+
if has_partial_output_writes(module, func) {
863+
err = err.with_note(
864+
"detected partial component writes (e.g., `out.x = value`) which were optimized away"
865+
).with_help(
866+
"write all components at once: `*out_frag_color = vec4(r, g, b, 1.0)`"
867+
);
868+
}
869+
}
870+
871+
return Err(err.emit());
872+
}
873+
}
874+
}
875+
}
876+
}
877+
878+
Ok(())
879+
}
880+
881+
/// Check if a fragment shader function has any meaningful output writes
882+
fn fragment_shader_writes_output(module: &Module, function_id: Word) -> bool {
883+
// Find the function definition
884+
let function = match module
885+
.functions
886+
.iter()
887+
.find(|f| f.def_id() == Some(function_id))
888+
{
889+
Some(func) => func,
890+
None => return true, // If function not found, assume it's valid
891+
};
892+
893+
// Check all instructions in the function for output writes
894+
for block in &function.blocks {
895+
for inst in &block.instructions {
896+
if inst.class.opcode == Op::Store {
897+
if let Some(target_id) = inst.operands.first().and_then(|op| op.id_ref_any()) {
898+
if is_output_storage_variable(module, target_id) {
899+
return true;
900+
}
901+
}
902+
}
903+
904+
// Check function calls recursively
905+
if inst.class.opcode == Op::FunctionCall {
906+
if let Some(callee_id) = inst.operands.first().and_then(|op| op.id_ref_any()) {
907+
if fragment_shader_writes_output(module, callee_id) {
908+
return true;
909+
}
910+
}
911+
}
912+
}
913+
}
914+
915+
false
916+
}
917+
918+
/// Check if a fragment shader has partial output writes that might have been optimized away
919+
fn has_partial_output_writes(module: &Module, function: &Function) -> bool {
920+
use rspirv::spirv::Op;
921+
922+
// Look for AccessChain operations on output variables
923+
// This suggests the shader was trying to write individual components
924+
for block in &function.blocks {
925+
for inst in &block.instructions {
926+
if matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain) {
927+
if let Some(base_id) = inst.operands.first().and_then(|op| op.id_ref_any()) {
928+
if is_output_storage_variable(module, base_id) {
929+
return true;
930+
}
931+
}
932+
}
933+
}
934+
}
935+
936+
false
937+
}
938+
939+
/// Check if a variable ID refers to an Output storage class variable
940+
fn is_output_storage_variable(module: &Module, var_id: Word) -> bool {
941+
use rspirv::spirv::{Op, StorageClass};
942+
943+
// Check direct output variables
944+
for inst in &module.types_global_values {
945+
if inst.result_id == Some(var_id) && inst.class.opcode == Op::Variable {
946+
if let Some(storage_class) = inst.operands.first() {
947+
return storage_class.unwrap_storage_class() == StorageClass::Output;
948+
}
949+
}
950+
}
951+
952+
// Check if this is a pointer derived from an output variable
953+
for inst in &module.types_global_values {
954+
if inst.result_id == Some(var_id)
955+
&& matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain)
956+
{
957+
if let Some(base_id) = inst.operands.first().and_then(|op| op.id_ref_any()) {
958+
return is_output_storage_variable(module, base_id);
959+
}
960+
}
961+
}
962+
963+
// Check in function bodies for derived pointers
964+
for function in &module.functions {
965+
for block in &function.blocks {
966+
for inst in &block.instructions {
967+
if inst.result_id == Some(var_id)
968+
&& matches!(inst.class.opcode, Op::AccessChain | Op::InBoundsAccessChain)
969+
{
970+
if let Some(base_id) = inst.operands.first().and_then(|op| op.id_ref_any()) {
971+
return is_output_storage_variable(module, base_id);
972+
}
973+
}
974+
}
975+
}
976+
}
977+
978+
false
979+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//@ compile-flags: --crate-type dylib --emit=metadata
2+
// Test for issue #284: A fragment shader that produces no output should fail
3+
4+
use spirv_std::spirv;
5+
6+
#[spirv(fragment)]
7+
pub fn main_fs(#[spirv(flat)] in_color: u32, out_frag_color: &mut spirv_std::glam::Vec4) {
8+
// This fragment shader reads input but doesn't write output
9+
// The assignment is optimized away, causing no visible output
10+
let _temp = in_color;
11+
// Note: No assignment to out_frag_color, so this shader produces no output
12+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: fragment shader `main_fs` produces no output
2+
|
3+
= help: fragment shaders must write to output parameters to produce visible results
4+
= note: use complete assignment like `*out_frag_color = vec4(r, g, b, a)` instead of partial component assignments
5+
= note: partial component assignments may be optimized away if not all components are written
6+
7+
error: aborting due to 1 previous error
8+

0 commit comments

Comments
 (0)