From 0ac78d2a8f7f37af2e015b75a2893c858c73c0d7 Mon Sep 17 00:00:00 2001 From: firestar99 Date: Mon, 3 Nov 2025 11:02:05 +0100 Subject: [PATCH 1/4] shared: compiletest for unused shared memory, failing --- .../compiletests/ui/arch/shared/dce_shared.rs | 36 ++++++++++++++ .../ui/arch/shared/dce_shared.stderr | 49 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 tests/compiletests/ui/arch/shared/dce_shared.rs create mode 100644 tests/compiletests/ui/arch/shared/dce_shared.stderr diff --git a/tests/compiletests/ui/arch/shared/dce_shared.rs b/tests/compiletests/ui/arch/shared/dce_shared.rs new file mode 100644 index 0000000000..2c639ae5fc --- /dev/null +++ b/tests/compiletests/ui/arch/shared/dce_shared.rs @@ -0,0 +1,36 @@ +// build-pass +// compile-flags: -C llvm-args=--disassemble-globals +// normalize-stderr-test "OpSource .*\n" -> "" +// normalize-stderr-test "%\d+ = OpString .*\n" -> "" +// normalize-stderr-test "OpMemoryModel Logical Vulkan" -> "OpMemoryModel Logical Simple" +// normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> "" +// ignore-spv1.0 +// ignore-spv1.1 +// ignore-spv1.2 +// ignore-spv1.3 +// ignore-vulkan1.0 +// ignore-vulkan1.1 + +use spirv_std::arch::workgroup_memory_barrier_with_group_sync; +use spirv_std::glam::*; +use spirv_std::spirv; + +#[spirv(compute(threads(2)))] +pub fn main( + #[spirv(descriptor_set = 0, binding = 0, storage_buffer)] input: &f32, + #[spirv(descriptor_set = 0, binding = 1, storage_buffer)] output: &mut f32, + #[spirv(workgroup)] used_shared: &mut f32, + #[spirv(workgroup)] dce_shared: &mut [i32; 2], + #[spirv(local_invocation_index)] inv_id: UVec3, +) { + unsafe { + let inv_id = inv_id.x as usize; + if inv_id == 0 { + *used_shared = *input; + } + workgroup_memory_barrier_with_group_sync(); + if inv_id == 1 { + *output = *used_shared; + } + } +} diff --git a/tests/compiletests/ui/arch/shared/dce_shared.stderr b/tests/compiletests/ui/arch/shared/dce_shared.stderr new file mode 100644 index 0000000000..8e21b8fe12 --- /dev/null +++ b/tests/compiletests/ui/arch/shared/dce_shared.stderr @@ -0,0 +1,49 @@ +OpCapability Shader +OpMemoryModel Logical Simple +OpEntryPoint GLCompute %1 "main" %2 %3 %4 %5 %6 +OpExecutionMode %1 LocalSize 2 1 1 +OpName %2 "input" +OpName %3 "output" +OpName %6 "inv_id" +OpName %4 "used_shared" +OpName %5 "dce_shared" +OpDecorate %9 Block +OpMemberDecorate %9 0 Offset 0 +OpDecorate %10 ArrayStride 4 +OpDecorate %2 NonWritable +OpDecorate %2 Binding 0 +OpDecorate %2 DescriptorSet 0 +OpDecorate %3 Binding 1 +OpDecorate %3 DescriptorSet 0 +OpDecorate %6 BuiltIn LocalInvocationIndex +%11 = OpTypeFloat 32 +%9 = OpTypeStruct %11 +%12 = OpTypePointer StorageBuffer %9 +%13 = OpTypePointer Workgroup %11 +%14 = OpTypeInt 32 1 +%15 = OpTypeInt 32 0 +%16 = OpConstant %15 2 +%10 = OpTypeArray %14 %16 +%17 = OpTypePointer Workgroup %10 +%18 = OpTypeVector %15 3 +%19 = OpTypePointer Input %18 +%20 = OpTypeVoid +%21 = OpTypeFunction %20 +%22 = OpTypePointer StorageBuffer %11 +%2 = OpVariable %12 StorageBuffer +%23 = OpConstant %15 0 +%3 = OpVariable %12 StorageBuffer +%6 = OpVariable %19 Input +%24 = OpTypeBool +%4 = OpVariable %13 Workgroup +%25 = OpConstant %15 264 +%26 = OpConstant %15 1 +%5 = OpVariable %17 Workgroup +error: error:0:0 - [VUID-StandaloneSpirv-None-10684] Invalid explicit layout decorations on type for operand '17[%_ptr_Workgroup__arr_int_uint_2]' + %dce_shared = OpVariable %_ptr_Workgroup__arr_int_uint_2 Workgroup + | + = note: spirv-val failed + = note: module `$TEST_BUILD_DIR/arch/shared/dce_shared.vulkan1.2` + +error: aborting due to 1 previous error + From f4ec5cace092798523dd4977edead5632f7efabd Mon Sep 17 00:00:00 2001 From: firestar99 Date: Mon, 3 Nov 2025 10:40:01 +0100 Subject: [PATCH 2/4] shared: fix unused shared memory failing validation --- .../linker/spirt_passes/explicit_layout.rs | 7 +-- .../ui/arch/shared/dce_shared.stderr | 43 ++++++++----------- 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/explicit_layout.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/explicit_layout.rs index 2812e452be..fc509bea44 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/explicit_layout.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/explicit_layout.rs @@ -41,12 +41,7 @@ pub fn erase_when_invalid(module: &mut Module) { parent_block: None, }; - // Seed the queues starting from the module exports. - for exportee in module.exports.values_mut() { - exportee - .inner_transform_with(&mut eraser) - .apply_to(exportee); - } + eraser.in_place_transform_module(module); // Process the queues until they're all empty. while !eraser.global_var_queue.is_empty() || !eraser.func_queue.is_empty() { diff --git a/tests/compiletests/ui/arch/shared/dce_shared.stderr b/tests/compiletests/ui/arch/shared/dce_shared.stderr index 8e21b8fe12..0884f622f8 100644 --- a/tests/compiletests/ui/arch/shared/dce_shared.stderr +++ b/tests/compiletests/ui/arch/shared/dce_shared.stderr @@ -9,41 +9,32 @@ OpName %4 "used_shared" OpName %5 "dce_shared" OpDecorate %9 Block OpMemberDecorate %9 0 Offset 0 -OpDecorate %10 ArrayStride 4 OpDecorate %2 NonWritable OpDecorate %2 Binding 0 OpDecorate %2 DescriptorSet 0 OpDecorate %3 Binding 1 OpDecorate %3 DescriptorSet 0 OpDecorate %6 BuiltIn LocalInvocationIndex -%11 = OpTypeFloat 32 -%9 = OpTypeStruct %11 -%12 = OpTypePointer StorageBuffer %9 -%13 = OpTypePointer Workgroup %11 -%14 = OpTypeInt 32 1 -%15 = OpTypeInt 32 0 -%16 = OpConstant %15 2 -%10 = OpTypeArray %14 %16 -%17 = OpTypePointer Workgroup %10 -%18 = OpTypeVector %15 3 +%10 = OpTypeFloat 32 +%9 = OpTypeStruct %10 +%11 = OpTypePointer StorageBuffer %9 +%12 = OpTypePointer Workgroup %10 +%13 = OpTypeInt 32 1 +%14 = OpTypeInt 32 0 +%15 = OpConstant %14 2 +%16 = OpTypeArray %13 %15 +%17 = OpTypePointer Workgroup %16 +%18 = OpTypeVector %14 3 %19 = OpTypePointer Input %18 %20 = OpTypeVoid %21 = OpTypeFunction %20 -%22 = OpTypePointer StorageBuffer %11 -%2 = OpVariable %12 StorageBuffer -%23 = OpConstant %15 0 -%3 = OpVariable %12 StorageBuffer +%22 = OpTypePointer StorageBuffer %10 +%2 = OpVariable %11 StorageBuffer +%23 = OpConstant %14 0 +%3 = OpVariable %11 StorageBuffer %6 = OpVariable %19 Input %24 = OpTypeBool -%4 = OpVariable %13 Workgroup -%25 = OpConstant %15 264 -%26 = OpConstant %15 1 +%4 = OpVariable %12 Workgroup +%25 = OpConstant %14 264 +%26 = OpConstant %14 1 %5 = OpVariable %17 Workgroup -error: error:0:0 - [VUID-StandaloneSpirv-None-10684] Invalid explicit layout decorations on type for operand '17[%_ptr_Workgroup__arr_int_uint_2]' - %dce_shared = OpVariable %_ptr_Workgroup__arr_int_uint_2 Workgroup - | - = note: spirv-val failed - = note: module `$TEST_BUILD_DIR/arch/shared/dce_shared.vulkan1.2` - -error: aborting due to 1 previous error - From e43d035b7f81bdee62c2abebd587bc537b9a5fbc Mon Sep 17 00:00:00 2001 From: firestar99 Date: Mon, 3 Nov 2025 14:56:06 +0100 Subject: [PATCH 3/4] shared: move `preserve-bindings` to linker Options --- crates/rustc_codegen_spirv/src/codegen_cx/mod.rs | 8 +------- crates/rustc_codegen_spirv/src/link.rs | 2 +- crates/rustc_codegen_spirv/src/linker/mod.rs | 1 + 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index c8acc4bc1d..d36e789b1c 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs @@ -313,9 +313,6 @@ pub struct CodegenArgs { pub run_spirv_opt: bool, - // spirv-opt flags - pub preserve_bindings: bool, - /// All options pertinent to `rustc_codegen_spirv::linker` specifically. // // FIXME(eddyb) should these be handled as `-C linker-args="..."` instead? @@ -576,8 +573,6 @@ impl CodegenArgs { // FIXME(eddyb) clean up this `no-` "negation prefix" situation. let run_spirv_opt = !matches.opt_present("no-spirv-opt"); - let preserve_bindings = matches.opt_present("preserve-bindings"); - let relax_block_layout = if relax_block_layout { Some(true) } else { None }; let spirv_metadata = match spirv_metadata.as_deref() { @@ -607,6 +602,7 @@ impl CodegenArgs { early_report_zombies: !matches.opt_present("no-early-report-zombies"), infer_storage_classes: !matches.opt_present("no-infer-storage-classes"), structurize: !matches.opt_present("no-structurize"), + preserve_bindings: matches.opt_present("preserve-bindings"), spirt_passes: matches .opt_strs("spirt-passes") .iter() @@ -656,8 +652,6 @@ impl CodegenArgs { run_spirv_opt, - preserve_bindings, - linker_opts, // NOTE(eddyb) these are debugging options that used to be env vars diff --git a/crates/rustc_codegen_spirv/src/link.rs b/crates/rustc_codegen_spirv/src/link.rs index 30c45f433a..038d307930 100644 --- a/crates/rustc_codegen_spirv/src/link.rs +++ b/crates/rustc_codegen_spirv/src/link.rs @@ -279,7 +279,7 @@ fn post_link_single_module( let opt_options = spirv_tools::opt::Options { validator_options: Some(val_options.clone()), max_id_bound: None, - preserve_bindings: cg_args.preserve_bindings, + preserve_bindings: cg_args.linker_opts.preserve_bindings, preserve_spec_constants: false, }; diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index d289c8e5fc..4cfa95d4d6 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -42,6 +42,7 @@ pub struct Options { pub early_report_zombies: bool, pub infer_storage_classes: bool, pub structurize: bool, + pub preserve_bindings: bool, pub spirt_passes: Vec, pub abort_strategy: Option, From 6d98265b74c3827aa4617e5e4bc67d13f3952395 Mon Sep 17 00:00:00 2001 From: firestar99 Date: Mon, 3 Nov 2025 14:57:40 +0100 Subject: [PATCH 4/4] shared: only retain entry point vars with `preserve-bindings` --- .../src/linker/entry_interface.rs | 17 +++-- crates/rustc_codegen_spirv/src/linker/mod.rs | 2 +- .../ui/arch/shared/dce_shared.stderr | 55 +++++++-------- .../ui/dis/issue-723-output.stderr | 15 ++-- .../ui/dis/scalars.supported.stderr | 70 +++++++++---------- 5 files changed, 76 insertions(+), 83 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/linker/entry_interface.rs b/crates/rustc_codegen_spirv/src/linker/entry_interface.rs index 3e490466e8..98382a0882 100644 --- a/crates/rustc_codegen_spirv/src/linker/entry_interface.rs +++ b/crates/rustc_codegen_spirv/src/linker/entry_interface.rs @@ -13,7 +13,7 @@ type Id = Word; /// /// This is needed for (arguably-not-interface) `Private` in SPIR-V >= 1.4, /// but also any interface variables declared "out of band" (e.g. via `asm!`). -pub fn gather_all_interface_vars_from_uses(module: &mut Module) { +pub fn gather_all_interface_vars_from_uses(module: &mut Module, preserve_bindings: bool) { // Start by mapping out which global (i.e. `OpVariable` or constants) IDs // can be used to access any interface-relevant `OpVariable`s // (where "interface-relevant" depends on the version, see comments below). @@ -87,12 +87,15 @@ pub fn gather_all_interface_vars_from_uses(module: &mut Module) { entry.operands[1].unwrap_id_ref() ); - // NOTE(eddyb) it might be better to remove any unused vars, or warn - // the user about their presence, but for now this keeps them around. - let mut interface_vars: IndexSet = entry.operands[3..] - .iter() - .map(|operand| operand.unwrap_id_ref()) - .collect(); + // `preserve_bindings` retains any declared entry point vars, otherwise we may DCE them + let mut interface_vars: IndexSet = if preserve_bindings { + entry.operands[3..] + .iter() + .map(|operand| operand.unwrap_id_ref()) + .collect() + } else { + IndexSet::default() + }; interface_vars.extend(&used_vars_per_fn_idx[entry_func_idx]); diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 4cfa95d4d6..908f861307 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -654,7 +654,7 @@ pub fn link( { let _timer = sess.timer("link_gather_all_interface_vars_from_uses"); - entry_interface::gather_all_interface_vars_from_uses(&mut output); + entry_interface::gather_all_interface_vars_from_uses(&mut output, opts.preserve_bindings); } if opts.spirv_metadata == SpirvMetadata::NameVariables { diff --git a/tests/compiletests/ui/arch/shared/dce_shared.stderr b/tests/compiletests/ui/arch/shared/dce_shared.stderr index 0884f622f8..532063d2ac 100644 --- a/tests/compiletests/ui/arch/shared/dce_shared.stderr +++ b/tests/compiletests/ui/arch/shared/dce_shared.stderr @@ -1,40 +1,35 @@ OpCapability Shader OpMemoryModel Logical Simple -OpEntryPoint GLCompute %1 "main" %2 %3 %4 %5 %6 +OpEntryPoint GLCompute %1 "main" %2 %3 %4 %5 OpExecutionMode %1 LocalSize 2 1 1 OpName %2 "input" OpName %3 "output" -OpName %6 "inv_id" -OpName %4 "used_shared" -OpName %5 "dce_shared" -OpDecorate %9 Block -OpMemberDecorate %9 0 Offset 0 +OpName %4 "inv_id" +OpName %5 "used_shared" +OpDecorate %8 Block +OpMemberDecorate %8 0 Offset 0 OpDecorate %2 NonWritable OpDecorate %2 Binding 0 OpDecorate %2 DescriptorSet 0 OpDecorate %3 Binding 1 OpDecorate %3 DescriptorSet 0 -OpDecorate %6 BuiltIn LocalInvocationIndex -%10 = OpTypeFloat 32 -%9 = OpTypeStruct %10 -%11 = OpTypePointer StorageBuffer %9 -%12 = OpTypePointer Workgroup %10 -%13 = OpTypeInt 32 1 -%14 = OpTypeInt 32 0 -%15 = OpConstant %14 2 -%16 = OpTypeArray %13 %15 -%17 = OpTypePointer Workgroup %16 -%18 = OpTypeVector %14 3 -%19 = OpTypePointer Input %18 -%20 = OpTypeVoid -%21 = OpTypeFunction %20 -%22 = OpTypePointer StorageBuffer %10 -%2 = OpVariable %11 StorageBuffer -%23 = OpConstant %14 0 -%3 = OpVariable %11 StorageBuffer -%6 = OpVariable %19 Input -%24 = OpTypeBool -%4 = OpVariable %12 Workgroup -%25 = OpConstant %14 264 -%26 = OpConstant %14 1 -%5 = OpVariable %17 Workgroup +OpDecorate %4 BuiltIn LocalInvocationIndex +%9 = OpTypeFloat 32 +%8 = OpTypeStruct %9 +%10 = OpTypePointer StorageBuffer %8 +%11 = OpTypePointer Workgroup %9 +%12 = OpTypeInt 32 0 +%13 = OpConstant %12 2 +%14 = OpTypeVector %12 3 +%15 = OpTypePointer Input %14 +%16 = OpTypeVoid +%17 = OpTypeFunction %16 +%18 = OpTypePointer StorageBuffer %9 +%2 = OpVariable %10 StorageBuffer +%19 = OpConstant %12 0 +%3 = OpVariable %10 StorageBuffer +%4 = OpVariable %15 Input +%20 = OpTypeBool +%5 = OpVariable %11 Workgroup +%21 = OpConstant %12 264 +%22 = OpConstant %12 1 diff --git a/tests/compiletests/ui/dis/issue-723-output.stderr b/tests/compiletests/ui/dis/issue-723-output.stderr index 093a9b89d1..690d7463d9 100644 --- a/tests/compiletests/ui/dis/issue-723-output.stderr +++ b/tests/compiletests/ui/dis/issue-723-output.stderr @@ -1,13 +1,8 @@ OpCapability Shader OpMemoryModel Logical Simple -OpEntryPoint Fragment %1 "main" %2 +OpEntryPoint Fragment %1 "main" OpExecutionMode %1 OriginUpperLeft -%3 = OpString "$DIR/issue-723-output.rs" -OpName %4 "issue_723_output::main" -OpDecorate %2 Location 0 -%5 = OpTypeFloat 32 -%6 = OpTypeVector %5 4 -%7 = OpTypePointer Output %6 -%8 = OpTypeVoid -%9 = OpTypeFunction %8 -%2 = OpVariable %7 Output +%2 = OpString "$DIR/issue-723-output.rs" +OpName %3 "issue_723_output::main" +%4 = OpTypeVoid +%5 = OpTypeFunction %4 diff --git a/tests/compiletests/ui/dis/scalars.supported.stderr b/tests/compiletests/ui/dis/scalars.supported.stderr index 595406ec94..3e31c257f7 100644 --- a/tests/compiletests/ui/dis/scalars.supported.stderr +++ b/tests/compiletests/ui/dis/scalars.supported.stderr @@ -7,37 +7,37 @@ OpMemoryModel Logical Simple OpEntryPoint Fragment %1 "main" %2 %3 %4 %5 %6 %7 %8 %9 %10 %11 %12 OpExecutionMode %1 OriginUpperLeft %13 = OpString "$DIR/scalars.rs" -OpName %3 "in_u8" -OpName %4 "in_u16" -OpName %5 "in_u32" -OpName %6 "in_u64" -OpName %7 "in_i8" -OpName %8 "in_i16" -OpName %9 "in_i32" -OpName %10 "in_i64" -OpName %11 "in_f32" -OpName %12 "in_f64" -OpName %2 "out" +OpName %2 "in_u8" +OpName %3 "in_u16" +OpName %4 "in_u32" +OpName %5 "in_u64" +OpName %6 "in_i8" +OpName %7 "in_i16" +OpName %8 "in_i32" +OpName %9 "in_i64" +OpName %10 "in_f32" +OpName %11 "in_f64" +OpName %12 "out" +OpDecorate %2 Flat +OpDecorate %2 Location 0 OpDecorate %3 Flat -OpDecorate %3 Location 0 +OpDecorate %3 Location 1 OpDecorate %4 Flat -OpDecorate %4 Location 1 +OpDecorate %4 Location 2 OpDecorate %5 Flat -OpDecorate %5 Location 2 +OpDecorate %5 Location 3 OpDecorate %6 Flat -OpDecorate %6 Location 3 +OpDecorate %6 Location 4 OpDecorate %7 Flat -OpDecorate %7 Location 4 +OpDecorate %7 Location 5 OpDecorate %8 Flat -OpDecorate %8 Location 5 +OpDecorate %8 Location 6 OpDecorate %9 Flat -OpDecorate %9 Location 6 -OpDecorate %10 Flat -OpDecorate %10 Location 7 -OpDecorate %11 Location 8 -OpDecorate %12 Flat -OpDecorate %12 Location 9 -OpDecorate %2 Location 0 +OpDecorate %9 Location 7 +OpDecorate %10 Location 8 +OpDecorate %11 Flat +OpDecorate %11 Location 9 +OpDecorate %12 Location 0 %14 = OpTypeInt 32 0 %15 = OpTypePointer Output %14 %16 = OpTypeInt 8 0 @@ -61,18 +61,18 @@ OpDecorate %2 Location 0 %34 = OpTypePointer Input %33 %35 = OpTypeVoid %36 = OpTypeFunction %35 -%3 = OpVariable %17 Input -%4 = OpVariable %19 Input -%5 = OpVariable %20 Input -%6 = OpVariable %22 Input -%7 = OpVariable %24 Input -%8 = OpVariable %26 Input -%9 = OpVariable %28 Input -%10 = OpVariable %30 Input -%11 = OpVariable %32 Input -%12 = OpVariable %34 Input +%2 = OpVariable %17 Input +%3 = OpVariable %19 Input +%4 = OpVariable %20 Input +%5 = OpVariable %22 Input +%6 = OpVariable %24 Input +%7 = OpVariable %26 Input +%8 = OpVariable %28 Input +%9 = OpVariable %30 Input +%10 = OpVariable %32 Input +%11 = OpVariable %34 Input %37 = OpConstant %16 8 -%2 = OpVariable %15 Output +%12 = OpVariable %15 Output %38 = OpConstant %18 16 %39 = OpConstant %14 32 %40 = OpConstant %21 64