Skip to content

Commit 84b14d8

Browse files
committed
Fix optimizer not updating module ID bound after optimization
The optimizer allocates new IDs for synthesized constants, phi nodes, and materialized expressions but never updates the module header's bound field. Since rspirv's assemble() uses the header bound as-is, the output SPIR-V had a stale bound that was too small, causing "id exceeds declared id bound" validation errors. Update output.header.bound to next_id after optimization completes. Add a regression test that verifies the bound covers all IDs.
1 parent 83c90d6 commit 84b14d8

1 file changed

Lines changed: 61 additions & 0 deletions

File tree

  • rust/spirv-tools-opt/src/direct

rust/spirv-tools-opt/src/direct/mod.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,6 +1534,15 @@ pub fn optimize_module_direct(module: &Module) -> Result<Module, EgglogOptError>
15341534
// Pass true_roots so that modules without side effects don't have everything removed
15351535
cleanup_module(&mut output, &id_aliases, &true_roots);
15361536

1537+
// Step 8: Update the module's ID bound to account for any new IDs allocated
1538+
// during optimization (synthesized constants, phi nodes, materialized expressions).
1539+
// rspirv's assemble() uses the header bound as-is, so we must update it here.
1540+
if let Some(ref mut header) = output.header {
1541+
if next_id > header.bound {
1542+
header.bound = next_id;
1543+
}
1544+
}
1545+
15371546
Ok(output)
15381547
}
15391548

@@ -2256,4 +2265,56 @@ mod tests {
22562265
let ret = Instruction::new(Op::Return, None, None, vec![]);
22572266
assert!(!is_optimizable(&ret));
22582267
}
2268+
2269+
#[test]
2270+
fn optimized_module_id_bound_covers_all_ids() {
2271+
use rspirv::binary::Assemble;
2272+
use rspirv::dr::Builder;
2273+
use rspirv::spirv::{
2274+
AddressingModel, Capability, ExecutionMode, ExecutionModel, FunctionControl,
2275+
MemoryModel,
2276+
};
2277+
2278+
// Build a module with arithmetic that will be constant-folded,
2279+
// potentially creating new IDs for synthesized constants.
2280+
let mut b = Builder::new();
2281+
b.capability(Capability::Shader);
2282+
b.memory_model(AddressingModel::Logical, MemoryModel::Simple);
2283+
let int = b.type_int(32, 0);
2284+
let func_ty = b.type_function(int, vec![]);
2285+
let func = b
2286+
.begin_function(int, None, FunctionControl::NONE, func_ty)
2287+
.unwrap();
2288+
let _ = b.begin_block(None).unwrap();
2289+
let c4 = b.constant_bit32(int, 4);
2290+
let c5 = b.constant_bit32(int, 5);
2291+
let c2 = b.constant_bit32(int, 2);
2292+
let add = b.i_add(int, None, c4, c5).expect("add");
2293+
let sub = b.i_sub(int, None, add, c2).expect("sub");
2294+
b.ret_value(sub).unwrap();
2295+
b.end_function().unwrap();
2296+
b.entry_point(ExecutionModel::GLCompute, func, "main", []);
2297+
b.execution_mode(func, ExecutionMode::LocalSize, [1, 1, 1]);
2298+
2299+
let module = b.module();
2300+
let optimized = optimize_module_direct(&module).expect("optimization should succeed");
2301+
2302+
// Verify the header bound covers all IDs in the module
2303+
let bound = optimized.header.as_ref().expect("header").bound;
2304+
let max_id = optimized
2305+
.all_inst_iter()
2306+
.filter_map(|inst| inst.result_id)
2307+
.max()
2308+
.unwrap_or(0);
2309+
assert!(
2310+
bound > max_id,
2311+
"ID bound ({bound}) must be greater than max used ID ({max_id})"
2312+
);
2313+
2314+
// Also verify the assembled output parses without error
2315+
let words = optimized.assemble();
2316+
let mut loader = rspirv::dr::Loader::new();
2317+
rspirv::binary::parse_words(&words, &mut loader)
2318+
.expect("optimized module should parse successfully");
2319+
}
22592320
}

0 commit comments

Comments
 (0)