spirv: more fixes and improvements

- Formatting.
- Improve `decorate` helper function to generate a decoration for a result-id.
- Reorder some functions in a more logical way

3 files changed, 132 insertions, 139 deletions

diff --git a/src/codegen/spirv.zig b/src/codegen/spirv.zig
index 37ebfe1f4f..add4464f4d 100644
--- a/src/codegen/spirv.zig
+++ b/src/codegen/spirv.zig
@@ -880,7 +880,9 @@ pub const DeclGen = struct {
             // TODO: We probably need to have a special implementation of this for the C abi.
             .ret_ptr => try self.airAlloc(inst),
             .block   => try self.airBlock(inst),
+
             .load    => try self.airLoad(inst),
+            .store      => return self.airStore(inst),
 
             .br         => return self.airBr(inst),
             .breakpoint => return,
@@ -891,7 +893,6 @@ pub const DeclGen = struct {
             .loop       => return self.airLoop(inst),
             .ret        => return self.airRet(inst),
             .ret_load   => return self.airRetLoad(inst),
-            .store      => return self.airStore(inst),
             .switch_br  => return self.airSwitchBr(inst),
             .unreach    => return self.airUnreach(),
 
@@ -964,13 +965,8 @@ pub const DeclGen = struct {
             33...64 => .{ .uint64 = mask_value },
             else => unreachable,
         };
-        // TODO: We should probably optimize these constants a bit.
-        const mask_id = self.spv.allocId();
-        try self.spv.sections.types_globals_constants.emit(self.spv.gpa, .OpConstant, .{
-            .id_result_type = ty_id,
-            .id_result = mask_id,
-            .value = mask_lit,
-        });
+        // TODO: We should probably optimize the amount of these constants a bit.
+        const mask_id = try self.spv.emitConstant(ty_id, mask_lit);
         const result_id = self.spv.allocId();
         try self.func.body.emit(self.spv.gpa, .OpBitwiseAnd, .{
             .id_result_type = ty_id,
@@ -1119,13 +1115,11 @@ pub const DeclGen = struct {
         // Finally, construct the Zig type.
         // Layout is result, overflow.
         const result_id = self.spv.allocId();
+        const constituents = [_]IdRef{ result, casted_overflow };
         try self.func.body.emit(self.spv.gpa, .OpCompositeConstruct, .{
             .id_result_type = result_type_id,
             .id_result = result_id,
-            .constituents = &.{
-                result,
-                casted_overflow,
-            },
+            .constituents = &constituents,
         });
         return result_id;
     }
@@ -1277,11 +1271,12 @@ pub const DeclGen = struct {
 
     fn extractField(self: *DeclGen, result_ty: IdResultType, object: IdRef, field: u32) !IdRef {
         const result_id = self.spv.allocId();
+        const indexes = [_]u32{field};
         try self.func.body.emit(self.spv.gpa, .OpCompositeExtract, .{
             .id_result_type = result_ty,
             .id_result = result_id,
             .composite = object,
-            .indexes = &.{field},
+            .indexes = &indexes,
         });
         return result_id;
     }
@@ -1318,11 +1313,12 @@ pub const DeclGen = struct {
         };
 
         const result_id = self.spv.allocId();
+        const indexes = [_]IdRef{index};
         try self.func.body.emit(self.spv.gpa, .OpInBoundsAccessChain, .{
             .id_result_type = spv_ptr_ty,
             .id_result = result_id,
             .base = slice_ptr,
-            .indexes = &.{index},
+            .indexes = &indexes,
         });
         return result_id;
     }
@@ -1335,14 +1331,13 @@ pub const DeclGen = struct {
         const slice = try self.resolve(bin_op.lhs);
         const index = try self.resolve(bin_op.rhs);
 
-        const spv_elem_ty = try self.resolveTypeId(self.air.typeOfIndex(inst));
         var slice_buf: Type.SlicePtrFieldTypeBuffer = undefined;
-        const spv_ptr_ty = try self.resolveTypeId(slice_ty.slicePtrFieldType(&slice_buf));
+        const ptr_ty_id = try self.resolveTypeId(slice_ty.slicePtrFieldType(&slice_buf));
 
         const slice_ptr = blk: {
             const result_id = self.spv.allocId();
             try self.func.body.emit(self.spv.gpa, .OpCompositeExtract, .{
-                .id_result_type = spv_ptr_ty,
+                .id_result_type = ptr_ty_id,
                 .id_result = result_id,
                 .composite = slice,
                 .indexes = &.{0},
@@ -1352,22 +1347,17 @@ pub const DeclGen = struct {
 
         const elem_ptr = blk: {
             const result_id = self.spv.allocId();
+            const indexes = [_]IdRef{index};
             try self.func.body.emit(self.spv.gpa, .OpInBoundsAccessChain, .{
-                .id_result_type = spv_ptr_ty,
+                .id_result_type = ptr_ty_id,
                 .id_result = result_id,
                 .base = slice_ptr,
-                .indexes = &.{index},
+                .indexes = &indexes,
             });
             break :blk result_id;
         };
 
-        const result_id = self.spv.allocId();
-        try self.func.body.emit(self.spv.gpa, .OpLoad, .{
-            .id_result_type = spv_elem_ty,
-            .id_result = result_id,
-            .pointer = elem_ptr,
-        });
-        return result_id;
+        return try self.load(slice_ty, elem_ptr);
     }
 
     fn airPtrElemPtr(self: *DeclGen, inst: Air.Inst.Index) !?IdRef {
@@ -1386,11 +1376,12 @@ pub const DeclGen = struct {
         const rhs = try self.resolve(bin_op.rhs);
 
         const result_id = self.spv.allocId();
+        const indexes = [_]IdRef{rhs};
         try self.func.body.emit(self.spv.gpa, .OpInBoundsAccessChain, .{
             .id_result_type = result_type_id,
             .id_result = result_id,
             .base = base_ptr,
-            .indexes = &.{rhs},
+            .indexes = &indexes,
         });
         return result_id;
     }
@@ -1412,11 +1403,12 @@ pub const DeclGen = struct {
         assert(struct_ty.zigTypeTag() == .Struct); // Cannot do unions yet.
 
         const result_id = self.spv.allocId();
+        const indexes = [_]u32{field_index};
         try self.func.body.emit(self.spv.gpa, .OpCompositeExtract, .{
             .id_result_type = field_ty_id,
             .id_result = result_id,
             .composite = object,
-            .indexes = &.{field_index},
+            .indexes = &indexes,
         });
         return result_id;
     }
@@ -1433,20 +1425,16 @@ pub const DeclGen = struct {
             .Struct => switch (object_ty.containerLayout()) {
                 .Packed => unreachable, // TODO
                 else => {
-                    const field_index_id = self.spv.allocId();
                     const u32_ty_id = self.spv.typeResultId(try self.intType(.unsigned, 32));
-                    try self.spv.sections.types_globals_constants.emit(self.spv.gpa, .OpConstant, .{
-                        .id_result_type = u32_ty_id,
-                        .id_result = field_index_id,
-                        .value = .{ .uint32 = field_index },
-                    });
+                    const field_index_id = try self.spv.emitConstant(u32_ty_id, .{ .uint32 = field_index });
                     const result_id = self.spv.allocId();
                     const result_type_id = try self.resolveTypeId(result_ptr_ty);
+                    const indexes = [_]IdRef{field_index_id};
                     try self.func.body.emit(self.spv.gpa, .OpInBoundsAccessChain, .{
                         .id_result_type = result_type_id,
                         .id_result = result_id,
                         .base = object_ptr,
-                        .indexes = &.{field_index_id},
+                        .indexes = &indexes,
                     });
                     return result_id;
                 },
@@ -1591,28 +1579,51 @@ pub const DeclGen = struct {
         });
     }
 
-    fn airLoad(self: *DeclGen, inst: Air.Inst.Index) !IdRef {
+    fn airLoad(self: *DeclGen, inst: Air.Inst.Index) !?IdRef {
         const ty_op = self.air.instructions.items(.data)[inst].ty_op;
-        const operand_id = try self.resolve(ty_op.operand);
-        const ty = self.air.typeOfIndex(inst);
+        const ptr_ty = self.air.typeOf(ty_op.operand);
+        const operand = try self.resolve(ty_op.operand);
+        if (!ptr_ty.isVolatilePtr() and self.liveness.isUnused(inst)) return null;
 
-        const result_type_id = try self.resolveTypeId(ty);
-        const result_id = self.spv.allocId();
+        return try self.load(ptr_ty, operand);
+    }
 
+    fn load(self: *DeclGen, ptr_ty: Type, ptr: IdRef) !IdRef {
+        const value_ty = ptr_ty.childType();
+        const result_type_id = try self.resolveTypeId(value_ty);
+        const result_id = self.spv.allocId();
         const access = spec.MemoryAccess.Extended{
-            .Volatile = ty.isVolatilePtr(),
+            .Volatile = ptr_ty.isVolatilePtr(),
         };
-
         try self.func.body.emit(self.spv.gpa, .OpLoad, .{
             .id_result_type = result_type_id,
             .id_result = result_id,
-            .pointer = operand_id,
+            .pointer = ptr,
             .memory_access = access,
         });
-
         return result_id;
     }
 
+    fn airStore(self: *DeclGen, inst: Air.Inst.Index) !void {
+        const bin_op = self.air.instructions.items(.data)[inst].bin_op;
+        const ptr_ty = self.air.typeOf(bin_op.lhs);
+        const ptr = try self.resolve(bin_op.lhs);
+        const value = try self.resolve(bin_op.rhs);
+
+        try self.store(ptr_ty, ptr, value);
+    }
+
+    fn store(self: *DeclGen, ptr_ty: Type, ptr: IdRef, value: IdRef) !void {
+        const access = spec.MemoryAccess.Extended{
+            .Volatile = ptr_ty.isVolatilePtr(),
+        };
+        try self.func.body.emit(self.spv.gpa, .OpStore, .{
+            .pointer = ptr,
+            .object = value,
+            .memory_access = access,
+        });
+    }
+
     fn airLoop(self: *DeclGen, inst: Air.Inst.Index) !void {
         const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
         const loop = self.air.extraData(Air.Block, ty_pl.payload);
@@ -1644,7 +1655,6 @@ pub const DeclGen = struct {
         const un_op = self.air.instructions.items(.data)[inst].un_op;
         const ptr_ty = self.air.typeOf(un_op);
         const ret_ty = ptr_ty.childType();
-        const ret_ty_id = try self.resolveTypeId(ret_ty);
 
         if (!ret_ty.hasRuntimeBitsIgnoreComptime()) {
             try self.func.body.emit(self.spv.gpa, .OpReturn, {});
@@ -1652,29 +1662,9 @@ pub const DeclGen = struct {
         }
 
         const ptr = try self.resolve(un_op);
-        const result_id = self.spv.allocId();
-        try self.func.body.emit(self.spv.gpa, .OpLoad, .{
-            .id_result_type = ret_ty_id,
-            .id_result = result_id,
-            .pointer = ptr,
-        });
-        try self.func.body.emit(self.spv.gpa, .OpReturnValue, .{ .value = result_id });
-    }
-
-    fn airStore(self: *DeclGen, inst: Air.Inst.Index) !void {
-        const bin_op = self.air.instructions.items(.data)[inst].bin_op;
-        const dst_ptr_id = try self.resolve(bin_op.lhs);
-        const src_val_id = try self.resolve(bin_op.rhs);
-        const lhs_ty = self.air.typeOf(bin_op.lhs);
-
-        const access = spec.MemoryAccess.Extended{
-            .Volatile = lhs_ty.isVolatilePtr(),
-        };
-
-        try self.func.body.emit(self.spv.gpa, .OpStore, .{
-            .pointer = dst_ptr_id,
-            .object = src_val_id,
-            .memory_access = access,
+        const value = try self.load(ptr_ty, ptr);
+        try self.func.body.emit(self.spv.gpa, .OpReturnValue, .{
+            .value = value,
         });
     }
 
@@ -1691,7 +1681,7 @@ pub const DeclGen = struct {
                 const backing_bits = self.backingIntBits(bits) orelse {
                     return self.todo("implement composite int switch", .{});
                 };
-                break :blk if (backing_bits <= 32) 1 else 2;
+                break :blk if (backing_bits <= 32) @as(u32, 1) else 2;
             },
             .Enum => blk: {
                 var buffer: Type.Payload.Bits = undefined;
@@ -1700,7 +1690,7 @@ pub const DeclGen = struct {
                 const backing_bits = self.backingIntBits(int_info.bits) orelse {
                     return self.todo("implement composite int switch", .{});
                 };
-                break :blk if (backing_bits <= 32) 1 else 2;
+                break :blk if (backing_bits <= 32) @as(u32, 1) else 2;
             },
             else => return self.todo("implement switch for type {s}", .{@tagName(cond_ty.zigTypeTag())}), // TODO: Figure out which types apply here, and work around them as we can only do integers.
         };
diff --git a/src/codegen/spirv/Module.zig b/src/codegen/spirv/Module.zig
index 23d188deb3..1a6ac95a0a 100644
--- a/src/codegen/spirv/Module.zig
+++ b/src/codegen/spirv/Module.zig
@@ -253,7 +253,6 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType {
     const ref_id = result_id;
     const types = &self.sections.types_globals_constants;
     const debug_names = &self.sections.debug_names;
-    const annotations = &self.sections.annotations;
     const result_id_operand = .{ .id_result = result_id };
 
     switch (ty.tag()) {
@@ -355,13 +354,7 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType {
 
             const size_type = Type.initTag(.u32);
             const size_type_id = try self.resolveTypeId(size_type);
-
-            const length_id = self.allocId();
-            try types.emit(self.gpa, .OpConstant, .{
-                .id_result_type = size_type_id,
-                .id_result = length_id,
-                .value = .{ .uint32 = info.length },
-            });
+            const length_id = try self.emitConstant(size_type_id, .{ .uint32 = info.length });
 
             try types.emit(self.gpa, .OpTypeArray, .{
                 .id_result = result_id,
@@ -369,7 +362,7 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType {
                 .length = length_id,
             });
             if (info.array_stride != 0) {
-                try annotations.decorate(self.gpa, ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } });
+                try self.decorate(ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } });
             }
         },
         .runtime_array => {
@@ -379,7 +372,7 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType {
                 .element_type = self.typeResultId(ty.childType()),
             });
             if (info.array_stride != 0) {
-                try annotations.decorate(self.gpa, ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } });
+                try self.decorate(ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } });
             }
         },
         .@"struct" => {
@@ -403,13 +396,13 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType {
                 .type = self.typeResultId(ty.childType()),
             });
             if (info.array_stride != 0) {
-                try annotations.decorate(self.gpa, ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } });
+                try self.decorate(ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } });
             }
             if (info.alignment) |alignment| {
-                try annotations.decorate(self.gpa, ref_id, .{ .Alignment = .{ .alignment = alignment } });
+                try self.decorate(ref_id, .{ .Alignment = .{ .alignment = alignment } });
             }
             if (info.max_byte_offset) |max_byte_offset| {
-                try annotations.decorate(self.gpa, ref_id, .{ .MaxByteOffset = .{ .max_byte_offset = max_byte_offset } });
+                try self.decorate(ref_id, .{ .MaxByteOffset = .{ .max_byte_offset = max_byte_offset } });
             }
         },
         .function => {
@@ -438,7 +431,6 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType {
 
 fn decorateStruct(self: *Module, target: IdRef, info: *const Type.Payload.Struct) !void {
     const debug_names = &self.sections.debug_names;
-    const annotations = &self.sections.annotations;
 
     if (info.name.len != 0) {
         try debug_names.emit(self.gpa, .OpName, .{
@@ -449,15 +441,15 @@ fn decorateStruct(self: *Module, target: IdRef, info: *const Type.Payload.Struct
 
     // Decorations for the struct type itself.
     if (info.decorations.block)
-        try annotations.decorate(self.gpa, target, .Block);
+        try self.decorate(target, .Block);
     if (info.decorations.buffer_block)
-        try annotations.decorate(self.gpa, target, .BufferBlock);
+        try self.decorate(target, .BufferBlock);
     if (info.decorations.glsl_shared)
-        try annotations.decorate(self.gpa, target, .GLSLShared);
+        try self.decorate(target, .GLSLShared);
     if (info.decorations.glsl_packed)
-        try annotations.decorate(self.gpa, target, .GLSLPacked);
+        try self.decorate(target, .GLSLPacked);
     if (info.decorations.c_packed)
-        try annotations.decorate(self.gpa, target, .CPacked);
+        try self.decorate(target, .CPacked);
 
     // Decorations for the struct members.
     const extra = info.member_decoration_extra;
@@ -476,8 +468,7 @@ fn decorateStruct(self: *Module, target: IdRef, info: *const Type.Payload.Struct
 
         switch (member.offset) {
             .none => {},
-            else => try annotations.decorateMember(
-                self.gpa,
+            else => try self.decorateMember(
                 target,
                 index,
                 .{ .Offset = .{ .byte_offset = @enumToInt(member.offset) } },
@@ -485,70 +476,70 @@ fn decorateStruct(self: *Module, target: IdRef, info: *const Type.Payload.Struct
         }
 
         switch (d.matrix_layout) {
-            .row_major => try annotations.decorateMember(self.gpa, target, index, .RowMajor),
-            .col_major => try annotations.decorateMember(self.gpa, target, index, .ColMajor),
+            .row_major => try self.decorateMember(target, index, .RowMajor),
+            .col_major => try self.decorateMember(target, index, .ColMajor),
             .none => {},
         }
         if (d.matrix_layout != .none) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .MatrixStride = .{ .matrix_stride = extra[extra_i] },
             });
             extra_i += 1;
         }
 
         if (d.no_perspective)
-            try annotations.decorateMember(self.gpa, target, index, .NoPerspective);
+            try self.decorateMember(target, index, .NoPerspective);
         if (d.flat)
-            try annotations.decorateMember(self.gpa, target, index, .Flat);
+            try self.decorateMember(target, index, .Flat);
         if (d.patch)
-            try annotations.decorateMember(self.gpa, target, index, .Patch);
+            try self.decorateMember(target, index, .Patch);
         if (d.centroid)
-            try annotations.decorateMember(self.gpa, target, index, .Centroid);
+            try self.decorateMember(target, index, .Centroid);
         if (d.sample)
-            try annotations.decorateMember(self.gpa, target, index, .Sample);
+            try self.decorateMember(target, index, .Sample);
         if (d.invariant)
-            try annotations.decorateMember(self.gpa, target, index, .Invariant);
+            try self.decorateMember(target, index, .Invariant);
         if (d.@"volatile")
-            try annotations.decorateMember(self.gpa, target, index, .Volatile);
+            try self.decorateMember(target, index, .Volatile);
         if (d.coherent)
-            try annotations.decorateMember(self.gpa, target, index, .Coherent);
+            try self.decorateMember(target, index, .Coherent);
         if (d.non_writable)
-            try annotations.decorateMember(self.gpa, target, index, .NonWritable);
+            try self.decorateMember(target, index, .NonWritable);
         if (d.non_readable)
-            try annotations.decorateMember(self.gpa, target, index, .NonReadable);
+            try self.decorateMember(target, index, .NonReadable);
 
         if (d.builtin) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .BuiltIn = .{ .built_in = @intToEnum(spec.BuiltIn, extra[extra_i]) },
             });
             extra_i += 1;
         }
         if (d.stream) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .Stream = .{ .stream_number = extra[extra_i] },
             });
             extra_i += 1;
         }
         if (d.location) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .Location = .{ .location = extra[extra_i] },
             });
             extra_i += 1;
         }
         if (d.component) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .Component = .{ .component = extra[extra_i] },
             });
             extra_i += 1;
         }
         if (d.xfb_buffer) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .XfbBuffer = .{ .xfb_buffer_number = extra[extra_i] },
             });
             extra_i += 1;
         }
         if (d.xfb_stride) {
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .XfbStride = .{ .xfb_stride = extra[extra_i] },
             });
             extra_i += 1;
@@ -557,10 +548,50 @@ fn decorateStruct(self: *Module, target: IdRef, info: *const Type.Payload.Struct
             const len = extra[extra_i];
             extra_i += 1;
             const semantic = @ptrCast([*]const u8, &extra[extra_i])[0..len];
-            try annotations.decorateMember(self.gpa, target, index, .{
+            try self.decorateMember(target, index, .{
                 .UserSemantic = .{ .semantic = semantic },
             });
             extra_i += std.math.divCeil(u32, extra_i, @sizeOf(u32)) catch unreachable;
         }
     }
 }
+
+pub fn emitConstant(
+    self: *Module,
+    ty_id: spec.IdRef,
+    value: spec.LiteralContextDependentNumber,
+) !IdRef {
+    const result_id = self.allocId();
+    try self.sections.types_globals_constants.emit(self.gpa, .OpConstant, .{
+        .id_result_type = ty_id,
+        .id_result = result_id,
+        .value = value,
+    });
+    return result_id;
+}
+
+/// Decorate a result-id.
+pub fn decorate(
+    self: *Module,
+    target: spec.IdRef,
+    decoration: spec.Decoration.Extended,
+) !void {
+    try self.sections.annotations.emit(self.gpa, .OpDecorate, .{
+        .target = target,
+        .decoration = decoration,
+    });
+}
+
+/// Decorate a result-id which is a member of some struct.
+pub fn decorateMember(
+    self: *Module,
+    structure_type: spec.IdRef,
+    member: u32,
+    decoration: spec.Decoration.Extended,
+) !void {
+    try self.sections.annotations.emit(self.gpa, .OpMemberDecorate, .{
+        .structure_type = structure_type,
+        .member = member,
+        .decoration = decoration,
+    });
+}
diff --git a/src/codegen/spirv/Section.zig b/src/codegen/spirv/Section.zig
index 2fd8309261..ce40717ef3 100644
--- a/src/codegen/spirv/Section.zig
+++ b/src/codegen/spirv/Section.zig
@@ -65,34 +65,6 @@ pub fn emit(
     section.writeOperands(opcode.Operands(), operands);
 }
 
-/// Decorate a result-id.
-pub fn decorate(
-    section: *Section,
-    allocator: Allocator,
-    target: spec.IdRef,
-    decoration: spec.Decoration.Extended,
-) !void {
-    try section.emit(allocator, .OpDecorate, .{
-        .target = target,
-        .decoration = decoration,
-    });
-}
-
-/// Decorate a result-id which is a member of some struct.
-pub fn decorateMember(
-    section: *Section,
-    allocator: Allocator,
-    structure_type: spec.IdRef,
-    member: u32,
-    decoration: spec.Decoration.Extended,
-) !void {
-    try section.emit(allocator, .OpMemberDecorate, .{
-        .structure_type = structure_type,
-        .member = member,
-        .decoration = decoration,
-    });
-}
-
 pub fn writeWord(section: *Section, word: Word) void {
     section.instructions.appendAssumeCapacity(word);
 }