Skip to content

Commit c69d9ec

Browse files
committed
Fold OpLoad from Private variables with constant initializers
Disclosure: I had AI write this and I only superficially understand the code. Replace OpLoad from Private/Function storage class variables with their constant initializers. This eliminates pointer-to-pointer patterns like `&&123` which would otherwise generate invalid SPIR-V in Logical addressing mode (pointer-to-pointer is only allowed for StorageBuffer/Workgroup, not Private). The optimization runs after inlining (which may expose such patterns) and before DCE (so the now-unused variables can be removed). Also extend DCE to treat Private storage class variables as pure, allowing them to be removed when unused. Newer versions of spirv-val catch this, acusing compiletests to fail.
1 parent 5e1fe97 commit c69d9ec

3 files changed

Lines changed: 93 additions & 1 deletion

File tree

crates/rustc_codegen_spirv/src/linker/dce.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,15 @@ fn instruction_is_pure(inst: &Instruction) -> bool {
282282
| PtrEqual
283283
| PtrNotEqual
284284
| PtrDiff => true,
285-
Variable => inst.operands.first() == Some(&Operand::StorageClass(StorageClass::Function)),
285+
// Variables with Function or Private storage class are pure and can be DCE'd if unused.
286+
// Other storage classes (Input, Output, Uniform, etc.) are part of the shader interface
287+
// and must be kept.
288+
Variable => matches!(
289+
inst.operands.first(),
290+
Some(&Operand::StorageClass(
291+
StorageClass::Function | StorageClass::Private
292+
))
293+
),
286294
_ => false,
287295
}
288296
}

crates/rustc_codegen_spirv/src/linker/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,16 @@ pub fn link(
415415
inline::inline(sess, &mut output)?;
416416
}
417417

418+
// Fold OpLoad from Private/Function variables with constant initializers.
419+
// This must run after inlining (which may expose such patterns) and before DCE
420+
// (so that the now-unused variables can be removed).
421+
// This is critical for pointer-to-pointer patterns like `&&123` which would
422+
// otherwise generate invalid SPIR-V in Logical addressing mode.
423+
{
424+
let _timer = sess.timer("link_fold_load_from_constant_variable");
425+
peephole_opts::fold_load_from_constant_variable(&mut output);
426+
}
427+
418428
{
419429
let _timer = sess.timer("link_dce-after-inlining");
420430
dce::dce(&mut output);

crates/rustc_codegen_spirv/src/linker/peephole_opts.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,3 +606,77 @@ pub fn bool_fusion(
606606
}
607607
super::apply_rewrite_rules(&rewrite_rules, &mut function.blocks);
608608
}
609+
610+
/// Fold `OpLoad` from Private/Function storage class variables that have constant initializers.
611+
///
612+
/// This optimization handles patterns like:
613+
/// ```text
614+
/// %ptr = OpVariable %_ptr_Private_T Private %initializer
615+
/// %val = OpLoad %T %ptr
616+
/// ```
617+
/// After this optimization, uses of `%val` are replaced with `%initializer`.
618+
///
619+
/// This is particularly important for pointer-to-pointer constants (e.g., `&&123`)
620+
/// where the outer pointer variable has a known initializer (the inner pointer).
621+
/// Without this optimization, such patterns generate invalid SPIR-V in Logical
622+
/// addressing mode (pointer-to-pointer with Private storage class is not allowed).
623+
pub fn fold_load_from_constant_variable(module: &mut Module) {
624+
use rspirv::spirv::StorageClass;
625+
626+
// Build a map of variable ID -> initializer ID for Private/Function variables
627+
// that have constant initializers.
628+
let var_initializers: FxHashMap<Word, Word> = module
629+
.types_global_values
630+
.iter()
631+
.filter_map(|inst| {
632+
if inst.class.opcode != Op::Variable {
633+
return None;
634+
}
635+
// Check storage class - only Private and Function can be folded
636+
let storage_class = inst.operands.first()?.unwrap_storage_class();
637+
if !matches!(
638+
storage_class,
639+
StorageClass::Private | StorageClass::Function
640+
) {
641+
return None;
642+
}
643+
// Check for initializer (second operand after storage class)
644+
let initializer = inst.operands.get(1)?.id_ref_any()?;
645+
Some((inst.result_id?, initializer))
646+
})
647+
.collect();
648+
649+
if var_initializers.is_empty() {
650+
return;
651+
}
652+
653+
// Rewrite OpLoad instructions that load from variables with known initializers
654+
let mut rewrite_rules: FxHashMap<Word, Word> = FxHashMap::default();
655+
656+
for func in &mut module.functions {
657+
for block in &mut func.blocks {
658+
for inst in &mut block.instructions {
659+
if inst.class.opcode != Op::Load {
660+
continue;
661+
}
662+
// OpLoad has pointer as first operand
663+
let ptr_id = inst.operands[0].unwrap_id_ref();
664+
if let Some(&initializer) = var_initializers.get(&ptr_id) {
665+
// This load can be replaced with the initializer value
666+
if let Some(result_id) = inst.result_id {
667+
rewrite_rules.insert(result_id, initializer);
668+
// Turn this instruction into a Nop
669+
*inst = Instruction::new(Op::Nop, None, None, Vec::new());
670+
}
671+
}
672+
}
673+
}
674+
}
675+
676+
if !rewrite_rules.is_empty() {
677+
// Apply rewrite rules to all functions
678+
for func in &mut module.functions {
679+
super::apply_rewrite_rules(&rewrite_rules, &mut func.blocks);
680+
}
681+
}
682+
}

0 commit comments

Comments
 (0)