Commit 49a067ccfe

Robin Voetter <robin@voetter.nl>
2024-10-27 16:31:45
spirv: forbid merging logical pointers
Under some architecture/operating system combinations it is forbidden to return a pointer from a merge, as these pointers must point to a location at compile time. This adds a check for those cases when returning a pointer from a block merge.
1 parent 3901361
src/Sema.zig
@@ -6319,6 +6319,9 @@ fn resolveAnalyzedBlock(
     for (merges.results.items, merges.src_locs.items) |merge_inst, merge_src| {
         try sema.validateRuntimeValue(child_block, merge_src orelse src, merge_inst);
     }
+
+    try sema.checkMergeAllowed(child_block, type_src, resolved_ty);
+
     const ty_inst = Air.internedToRef(resolved_ty.toIntern());
     switch (block_tag) {
         .block => {
@@ -9754,6 +9757,39 @@ fn checkCallConvSupportsVarArgs(sema: *Sema, block: *Block, src: LazySrcLoc, cc:
     }
 }
 
+fn checkMergeAllowed(sema: *Sema, block: *Block, src: LazySrcLoc, peer_ty: Type) !void {
+    const pt = sema.pt;
+    const zcu = pt.zcu;
+    const target = zcu.getTarget();
+
+    if (!peer_ty.isPtrAtRuntime(zcu)) {
+        return;
+    }
+
+    const as = peer_ty.ptrAddressSpace(zcu);
+    if (!target_util.arePointersLogical(target, as)) {
+        return;
+    }
+
+    return sema.failWithOwnedErrorMsg(block, msg: {
+        const msg = try sema.errMsg(src, "value with non-mergable pointer type '{}' depends on runtime control flow", .{peer_ty.fmt(pt)});
+        errdefer msg.destroy(sema.gpa);
+
+        const runtime_src = block.runtime_cond orelse block.runtime_loop.?;
+        try sema.errNote(runtime_src, msg, "runtime control flow here", .{});
+
+        const backend = target_util.zigBackend(target, zcu.comp.config.use_llvm);
+        try sema.errNote(src, msg, "pointers with address space '{s}' cannot be returned from a branch on target {s}-{s} by compiler backend {s}", .{
+            @tagName(as),
+            target.cpu.arch.genericName(),
+            @tagName(target.os.tag),
+            @tagName(backend),
+        });
+
+        break :msg msg;
+    });
+}
+
 const Section = union(enum) {
     generic,
     default,
src/target.zig
@@ -398,6 +398,31 @@ pub fn addrSpaceCastIsValid(
     }
 }
 
+/// Under SPIR-V with Vulkan, pointers are not 'real' (physical), but rather 'logical'. Effectively,
+/// this means that all such pointers have to be resolvable to a location at compile time, and places
+/// a number of restrictions on usage of such pointers. For example, a logical pointer may not be
+/// part of a merge (result of a branch) and may not be stored in memory at all. This function returns
+/// for a particular architecture and address space wether such pointers are logical.
+pub fn arePointersLogical(target: std.Target, as: AddressSpace) bool {
+    if (target.os.tag != .vulkan) {
+        return false;
+    }
+
+    return switch (as) {
+        // TODO: Vulkan doesn't support pointers in the generic address space, we
+        // should remove this case but this requires a change in defaultAddressSpace().
+        // For now, at least disable them from being regarded as physical.
+        .generic => true,
+        // For now, all global pointers are represented using PhysicalStorageBuffer, so these are real
+        // pointers.
+        .global => false,
+        // TODO: Allowed with VK_KHR_variable_pointers.
+        .shared => true,
+        .constant, .local, .input, .output, .uniform => true,
+        else => unreachable,
+    };
+}
+
 pub fn llvmMachineAbi(target: std.Target) ?[:0]const u8 {
     // LLD does not support ELFv1. Rather than having LLVM produce ELFv1 code and then linking it
     // into a broken ELFv2 binary, just force LLVM to use ELFv2 as well. This will break when glibc
test/cases/compile_errors/spirv_merge_logical_pointers.zig
@@ -0,0 +1,19 @@
+export fn a() void {
+    var x: *i32 = undefined;
+    _ = &x;
+    var y: *i32 = undefined;
+    _ = &y;
+    var rt_cond = false;
+    _ = &rt_cond;
+
+    var z = if (rt_cond) x else y;
+    _ = &z;
+}
+
+// error
+// backend=stage2
+// target=spirv64-vulkan
+//
+// :9:13: error: value with non-mergable pointer type '*i32' depends on runtime control flow
+// :9:17: note: runtime control flow here
+// :9:13: note: pointers with address space 'generic' cannot be returned from a branch on target spirv-vulkan by compiler backend stage2_spirv64
test/cases/spirv_mergable_pointers.zig
@@ -0,0 +1,16 @@
+export fn a() void {
+    var x: *addrspace(.global) i32 = undefined;
+    _ = &x;
+    var y: *addrspace(.global) i32 = undefined;
+    _ = &y;
+    var rt_cond = false;
+    _ = &rt_cond;
+
+    var z = if (rt_cond) x else y;
+    _ = &z;
+}
+
+// compile
+// output_mode=Obj
+// backend=stage2
+// target=spirv64-vulkan
test/src/Cases.zig
@@ -467,7 +467,7 @@ fn addFromDirInner(
             const target = resolved_target.result;
             for (backends) |backend| {
                 if (backend == .stage2 and
-                    target.cpu.arch != .wasm32 and target.cpu.arch != .x86_64)
+                    target.cpu.arch != .wasm32 and target.cpu.arch != .x86_64 and target.cpu.arch != .spirv64)
                 {
                     // Other backends don't support new liveness format
                     continue;