Fix unused shared memory failing validation

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

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,
     };
 

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<Id> = 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<Id> = 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]);
 

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<String>,
 
     pub abort_strategy: Option<String>,
@@ -653,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 {

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() {

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;
+        }
+    }
+}

tests/compiletests/ui/arch/shared/dce_shared.stderr

@@ -0,0 +1,35 @@
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint GLCompute %1 "main" %2 %3 %4 %5
+OpExecutionMode %1 LocalSize 2 1 1
+OpName %2 "input"
+OpName %3 "output"
+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 %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

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

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