Do not merge zombie and non-zombies

crates/rustc_codegen_spirv/src/linker/duplicates.rs

@@ -95,6 +95,7 @@ fn gather_annotations(annotations: &[Instruction]) -> HashMap<Word, Vec<u32>> {
 fn make_type_key(
     inst: &Instruction,
     unresolved_forward_pointers: &HashSet<Word>,
+    zombies: &HashSet<Word>,
     annotations: &HashMap<Word, Vec<u32>>,
 ) -> Vec<u32> {
     let mut data = vec![];
@@ -120,6 +121,7 @@ fn make_type_key(
         }
     }
     if let Some(id) = inst.result_id {
+        data.push(if zombies.contains(&id) { 1 } else { 0 });
         if let Some(annos) = annotations.get(&id) {
             data.extend_from_slice(annos)
         }
@@ -140,10 +142,14 @@ fn rewrite_inst_with_rules(inst: &mut Instruction, rules: &HashMap<u32, u32>) {
     }
 }
 
-// TODO: Don't merge zombie types with non-zombie types
 pub fn remove_duplicate_types(module: &mut Module) {
     // Keep in mind, this algorithm requires forward type references to not exist - i.e. it's a valid spir-v module.
 
+    // Include zombies in the key to not merge zombies with non-zombies
+    let zombies: HashSet<Word> = super::zombies::collect_zombies(module)
+        .map(|(z, _)| z)
+        .collect();
+
     // When a duplicate type is encountered, then this is a map from the deleted ID, to the new, deduplicated ID.
     let mut rewrite_rules = HashMap::new();
     // Instructions are encoded into "keys": their opcode, followed by arguments, then annotations.
@@ -181,7 +187,7 @@ pub fn remove_duplicate_types(module: &mut Module) {
         // all_inst_iter_mut pass below. However, the code is a lil bit cleaner this way I guess.
         rewrite_inst_with_rules(inst, &rewrite_rules);
 
-        let key = make_type_key(inst, &unresolved_forward_pointers, &annotations);
+        let key = make_type_key(inst, &unresolved_forward_pointers, &zombies, &annotations);
 
         match key_to_result_id.entry(key) {
             hash_map::Entry::Vacant(entry) => {

crates/rustc_codegen_spirv/src/linker/import_export_link.rs

@@ -39,6 +39,7 @@ fn find_import_export_pairs_and_killed_params(
             return Err(ErrorReported);
         }
     }
+    let mut has_err = false;
     // Then, collect all the imports, and create the rewrite rules.
     for annotation in &module.annotations {
         let (import_id, name) = match get_linkage_inst(annotation) {
@@ -48,7 +49,8 @@ fn find_import_export_pairs_and_killed_params(
         let (export_id, export_type) = match exports.get(name) {
             None => {
                 sess.err(&format!("Unresolved symbol {:?}", name));
-                return Err(ErrorReported);
+                has_err = true;
+                continue;
             }
             Some(&x) => x,
         };
@@ -62,6 +64,9 @@ fn find_import_export_pairs_and_killed_params(
             }
         }
     }
+    if has_err {
+        return Err(ErrorReported);
+    }
 
     Ok((rewrite_rules, killed_parameters))
 }

crates/rustc_codegen_spirv/src/linker/zombies.rs

@@ -7,22 +7,18 @@ use std::collections::HashMap;
 use std::env;
 use std::iter::once;
 
-fn collect_zombies(module: &Module) -> Vec<(Word, String)> {
-    module
-        .annotations
-        .iter()
-        .filter_map(|inst| {
-            // TODO: Temp hack. We hijack UserTypeGOOGLE right now, since the compiler never emits this.
-            if inst.class.opcode == Op::DecorateString
-                && inst.operands[1].unwrap_decoration() == Decoration::UserTypeGOOGLE
-            {
-                let id = inst.operands[0].unwrap_id_ref();
-                let reason = inst.operands[2].unwrap_literal_string();
-                return Some((id, reason.to_string()));
-            }
-            None
-        })
-        .collect()
+pub fn collect_zombies(module: &Module) -> impl Iterator<Item = (Word, String)> + '_ {
+    module.annotations.iter().filter_map(|inst| {
+        // TODO: Temp hack. We hijack UserTypeGOOGLE right now, since the compiler never emits this.
+        if inst.class.opcode == Op::DecorateString
+            && inst.operands[1].unwrap_decoration() == Decoration::UserTypeGOOGLE
+        {
+            let id = inst.operands[0].unwrap_id_ref();
+            let reason = inst.operands[2].unwrap_literal_string();
+            return Some((id, reason.to_string()));
+        }
+        None
+    })
 }
 
 fn remove_zombie_annotations(module: &mut Module) {
@@ -161,7 +157,7 @@ fn report_error_zombies(sess: &Session, module: &Module, zombie: &HashMap<Word,
 }
 
 pub fn remove_zombies(sess: &Session, module: &mut Module) {
-    let zombies_owned = collect_zombies(module);
+    let zombies_owned = collect_zombies(module).collect::<Vec<_>>();
     let mut zombies = zombies_owned
         .iter()
         .map(|(a, b)| (*a, ZombieInfo::from_reason(b)))

crates/spirv-builder/src/test/basic.rs

@@ -97,6 +97,23 @@ OpFunctionEnd"#,
     );
 }
 
+// TODO: While the "checked sub is not implemented yet" issue is fixed and so this repro should be
+// fixed, a further underlying issue gets triggered instead, which is that the current structurizer
+// doesn't handle this case. Remove `#[ignore]` once fixed.
+#[test]
+#[ignore]
+fn logical_and() {
+    val(r#"
+fn f(x: bool, y: bool) -> bool {
+    x && y
+}
+#[allow(unused_attributes)]
+#[spirv(fragment)]
+pub fn main() {
+    f(false, true);
+}"#);
+}
+
 // TODO: Implement strings to make this compile
 #[test]
 #[ignore]