spirv: handle annotations in deduplication pass

src/link/SpirV/deduplicate.zig

@@ -44,6 +44,8 @@ const ModuleInfo = struct {
         /// or the entity that is affected by this entity if this entity
         /// is a decoration.
         result_id_index: u16,
+        /// The first decoration in `self.decorations`.
+        first_decoration: u32,
     };
 
     /// Maps result-id to Entity's
@@ -53,6 +55,8 @@ const ModuleInfo = struct {
     /// Because we need these values when recoding the module anyway,
     /// it contains the status of ALL operands in the module.
     operand_is_id: std.DynamicBitSetUnmanaged,
+    /// Store of decorations for each entity.
+    decorations: []const Entity,
 
     pub fn parse(
         arena: Allocator,
@@ -62,6 +66,7 @@ const ModuleInfo = struct {
         var entities = std.AutoArrayHashMap(ResultId, Entity).init(arena);
         var id_offsets = std.ArrayList(u16).init(arena);
         var operand_is_id = try std.DynamicBitSetUnmanaged.initEmpty(arena, binary.instructions.len);
+        var decorations = std.MultiArrayList(struct { target_id: ResultId, entity: Entity }){};
 
         var it = binary.iterateInstructions();
         while (it.next()) |inst| {
@@ -75,40 +80,99 @@ const ModuleInfo = struct {
 
             if (!canDeduplicate(inst.opcode)) continue;
 
+            // TODO: Is this robust enough? Maybe replace this with the result from
+            // the instruction spec, or add a function to the spec to get the
+            // result-id index or something... But DO take note that the result-id
+            // here also includes the ref-id of a debug instruction's target.
             const result_id_index: u16 = switch (inst.opcode.class()) {
                 .TypeDeclaration, .Annotation, .Debug => 0,
                 .ConstantCreation => 1,
                 else => unreachable,
             };
 
             const result_id: ResultId = @enumFromInt(inst.operands[id_offsets.items[result_id_index]]);
+            const entity = Entity{
+                .kind = inst.opcode,
+                .first_operand = first_operand_offset,
+                .num_operands = @intCast(inst.operands.len),
+                .result_id_index = result_id_index,
+                .first_decoration = undefined, // Filled in later
+            };
 
             switch (inst.opcode.class()) {
                 .Annotation, .Debug => {
-                    // TODO
+                    try decorations.append(arena, .{
+                        .target_id = result_id,
+                        .entity = entity,
+                    });
                 },
                 .TypeDeclaration, .ConstantCreation => {
                     const entry = try entities.getOrPut(result_id);
                     if (entry.found_existing) {
                         log.err("type or constant {} has duplicate definition", .{result_id});
                         return error.DuplicateId;
                     }
-                    entry.value_ptr.* = .{
-                        .kind = inst.opcode,
-                        .first_operand = first_operand_offset,
-                        .num_operands = @intCast(inst.operands.len),
-                        .result_id_index = result_id_index,
-                    };
+                    entry.value_ptr.* = entity;
                 },
                 else => unreachable,
             }
         }
 
+        // Sort decorations by the index of the result-id in `entities.
+        // This ensures not only that the decorations of a particular reuslt-id
+        // are continuous, but the subsequences also appear in the same order as in `entities`.
+
+        const SortContext = struct {
+            entities: std.AutoArrayHashMapUnmanaged(ResultId, Entity),
+            ids: []const ResultId,
+
+            pub fn lessThan(ctx: @This(), a_index: usize, b_index: usize) bool {
+                // If any index is not in the entities set, its because its not a
+                // deduplicatable result-id. Those should be considered largest and
+                // float to the end.
+                const entity_index_a = ctx.entities.getIndex(ctx.ids[a_index]) orelse return false;
+                const entity_index_b = ctx.entities.getIndex(ctx.ids[b_index]) orelse return true;
+
+                return entity_index_a < entity_index_b;
+            }
+        };
+
+        decorations.sort(SortContext{
+            .entities = entities.unmanaged,
+            .ids = decorations.items(.target_id),
+        });
+
+        // Now go through the decorations and add the offsets to the entities list.
+        var decoration_i: u32 = 0;
+        const target_ids = decorations.items(.target_id);
+        for (entities.keys(), entities.values()) |id, *entity| {
+            entity.first_decoration = decoration_i;
+
+            // Scan ahead to the next decoration
+            while (decoration_i < target_ids.len and target_ids[decoration_i] == id) {
+                decoration_i += 1;
+            }
+        }
+
         return ModuleInfo{
             .entities = entities.unmanaged,
             .operand_is_id = operand_is_id,
+            // There may be unrelated decorations at the end, so make sure to
+            // slice those off.
+            .decorations = decorations.items(.entity)[0..decoration_i],
         };
     }
+
+    fn entityDecorationsByIndex(self: ModuleInfo, index: usize) []const Entity {
+        const values = self.entities.values();
+        const first_decoration = values[index].first_decoration;
+        if (index == values.len - 1) {
+            return self.decorations[first_decoration..];
+        } else {
+            const next_first_decoration = values[index + 1].first_decoration;
+            return self.decorations[first_decoration..next_first_decoration];
+        }
+    }
 };
 
 const EntityContext = struct {
@@ -138,11 +202,17 @@ const EntityContext = struct {
         return hasher.final();
     }
 
-    fn hashInner(self: *EntityContext, hasher: *std.hash.Wyhash, id: ResultId) !void {
-        const index = self.info.entities.getIndex(id).?;
+    fn hashInner(self: *EntityContext, hasher: *std.hash.Wyhash, id: ResultId) error{OutOfMemory}!void {
+        const index = self.info.entities.getIndex(id) orelse {
+            // Index unknown, the type or constant may depend on another result-id
+            // that couldn't be deduplicated and so it wasn't added to info.entities.
+            // In this case, just has the ID itself.
+            std.hash.autoHash(hasher, id);
+            return;
+        };
+
         const entity = self.info.entities.values()[index];
 
-        std.hash.autoHash(hasher, entity.kind);
         if (entity.kind == .OpTypePointer) {
             // This may be either a pointer that is forward-referenced in the future,
             // or a forward reference to a pointer.
@@ -155,6 +225,17 @@ const EntityContext = struct {
             }
         }
 
+        try self.hashEntity(hasher, entity);
+
+        // Process decorations.
+        const decorations = self.info.entityDecorationsByIndex(index);
+        for (decorations) |decoration| {
+            try self.hashEntity(hasher, decoration);
+        }
+    }
+
+    fn hashEntity(self: *EntityContext, hasher: *std.hash.Wyhash, entity: ModuleInfo.Entity) !void {
+        std.hash.autoHash(hasher, entity.kind);
         // Process operands
         const operands = self.binary.instructions[entity.first_operand..][0..entity.num_operands];
         for (operands, 0..) |operand, i| {
@@ -178,19 +259,24 @@ const EntityContext = struct {
         return try self.eqlInner(a, b);
     }
 
-    fn eqlInner(self: *EntityContext, id_a: ResultId, id_b: ResultId) !bool {
-        const index_a = self.info.entities.getIndex(id_a).?;
-        const index_b = self.info.entities.getIndex(id_b).?;
+    fn eqlInner(self: *EntityContext, id_a: ResultId, id_b: ResultId) error{OutOfMemory}!bool {
+        const maybe_index_a = self.info.entities.getIndex(id_a);
+        const maybe_index_b = self.info.entities.getIndex(id_b);
+
+        if (maybe_index_a == null and maybe_index_b == null) {
+            // Both indices unknown. In this case the type or constant
+            // may depend on another result-id that couldn't be deduplicated
+            // (so it wasn't added to info.entities). In this case, that particular
+            // result-id should be the same one.
+            return id_a == id_b;
+        }
+
+        const index_a = maybe_index_a orelse return false;
+        const index_b = maybe_index_b orelse return false;
 
         const entity_a = self.info.entities.values()[index_a];
         const entity_b = self.info.entities.values()[index_b];
 
-        if (entity_a.kind != entity_b.kind) {
-            return false;
-        } else if (entity_a.result_id_index != entity_a.result_id_index) {
-            return false;
-        }
-
         if (entity_a.kind == .OpTypePointer) {
             // May be a forward reference, or should be saved as a potential
             // forward reference in the future. Whatever the case, it should
@@ -207,6 +293,33 @@ const EntityContext = struct {
             }
         }
 
+        if (!try self.eqlEntities(entity_a, entity_b)) {
+            return false;
+        }
+
+        // Compare decorations.
+        const decorations_a = self.info.entityDecorationsByIndex(index_a);
+        const decorations_b = self.info.entityDecorationsByIndex(index_b);
+        if (decorations_a.len != decorations_b.len) {
+            return false;
+        }
+
+        for (decorations_a, decorations_b) |decoration_a, decoration_b| {
+            if (!try self.eqlEntities(decoration_a, decoration_b)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    fn eqlEntities(self: *EntityContext, entity_a: ModuleInfo.Entity, entity_b: ModuleInfo.Entity) !bool {
+        if (entity_a.kind != entity_b.kind) {
+            return false;
+        } else if (entity_a.result_id_index != entity_a.result_id_index) {
+            return false;
+        }
+
         const operands_a = self.binary.instructions[entity_a.first_operand..][0..entity_a.num_operands];
         const operands_b = self.binary.instructions[entity_b.first_operand..][0..entity_b.num_operands];
 
@@ -312,8 +425,17 @@ pub fn run(parser: *BinaryModule.Parser, binary: *BinaryModule) !void {
                 new_functions_section = section.instructions.items.len;
             },
             .OpTypeForwardPointer => continue, // We re-emit these where needed
-            // TODO: These aren't supported yet, strip them out for testing purposes.
-            .OpName, .OpMemberName => continue,
+            else => {},
+        }
+
+        switch (inst.opcode.class()) {
+            // TODO: Make this check more robust?
+            .Annotation, .Debug => {
+                // For decoration-style instructions, only emit them
+                // if the target is not removed.
+                const target: ResultId = @enumFromInt(inst.operands[0]);
+                if (replace.contains(target)) continue;
+            },
             else => {},
         }