Skip to content

Commit 438295e

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 930e228 commit 438295e

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;
@@ -705,6 +705,9 @@ pub fn link(
705705
dce::dce(output);
706706
}
707707

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