Fix Metal buffer alias visibility and sparse copy ordering (#200)

Carry ResourceAlias visibility on copy encoders containing buffer copies,
inline updates or fills, using the contribution from PR #200. Operations
through sparse virtual mappings also need ordering against surrounding
operations: disjoint guest addresses can name the same physical bytes,
so guest-range dependency detection alone misses their hazards.

Add a regression using distinct no-copy Metal buffers and real sparse
mappings. The original host failed 37 of 101 checks on an M4 Pro and
PR #200 alone left 24 failures; the complete fix passes all 101 checks
with both direct and deferred encoding under Metal validation. Ordinary
independent buffer operations retain range-based batching.

Validation: t_nvn passes 154 checks; the Mac host builds; all 133 iOS
source compile checks and the documentation checks pass. Monster Hunter
Rise gameplay, iPhone execution and sparse-barrier performance were not
retested.

Co-authored-by: Aleksandr Svidchenkov <a.svidchenkov@gmail.com>

dm/gfx/nvn/hle_nvn.c

@@ -2187,6 +2187,16 @@ int nvn_host_readable(uint64_t a, uint64_t n) {
         break; }
     pthread_mutex_unlock(&g_vp_mu); return ok;
 }
+// Sparse virtual addresses alias physical pools, so guest ranges alone cannot
+// prove copy independence. Like nvn_sparse_pool, pool lifetime is fixed while used.
+int nvn_sparse_alias(uint64_t lo, uint64_t hi) {
+    if (hi <= lo) return 0;
+    for (int k = 0; k < NVPOOL; k++) { const struct vpool *v = &g_vp[k];
+        uint64_t base = (uint64_t)(uintptr_t)v->base;
+        if (base && lo < base + v->len && base < hi) return 1;
+    }
+    return 0;
+}
 // Textures of a sparse pool, read through its page table (see the sparse pools above): whether a pool is one, a copy of a texture's bytes (unmapped pages read as 0; → bytes not mapped),
 // whether its content may have changed since an upload (a mapping in its range newer than gen, or a physical page behind it written after the dirty-page stamp), and the write
 // protection of the physical pages behind it (taken before an upload, as for any packaged texture)

dm/gfx/nvn/nvn_mtl.m

@@ -655,6 +655,10 @@ static void zskip_clear(void);
 static int tbdr_on(void);
 static id g_cb;
 static id<MTL4ComputeCommandEncoder> g_blit;
+// Buffer operations may use distinct Metal GPU addresses for one allocation
+// (overlapping pools or sparse mappings). Their encoder barriers need alias cache visibility.
+static int g_blit_alias;
+static inline MTL4VisibilityOptions blit_vis(void) { return MTL4VisibilityOptionDevice | (g_blit_alias ? MTL4VisibilityOptionResourceAlias : 0); }
 static NSMutableArray<dispatch_block_t> *g_commit_blocks;
 static uint64_t g_alloc_fence[ALLOC_MAX]; static uint64_t g_alloc_waits, g_alloc_grown;
 int mtl_wait(uint64_t fence, uint64_t timeout_ns);
@@ -740,7 +744,8 @@ static uint64_t g_bar_skip, g_bar_full;            // draw passes begun without
 static void gp_end(id e);
 static inline void m4_end(id e) {
     gp_end(e);
-    if ([e conformsToProtocol:@protocol(MTL4CommandEncoder)]) [(id<MTL4CommandEncoder>)e barrierAfterStages:MTLStageAll beforeQueueStages:MTLStageAll visibilityOptions:MTL4VisibilityOptionDevice];
+    MTL4VisibilityOptions vis = e == g_blit ? blit_vis() : MTL4VisibilityOptionDevice; if (e == g_blit) g_blit_alias = 0;
+    if ([e conformsToProtocol:@protocol(MTL4CommandEncoder)]) [(id<MTL4CommandEncoder>)e barrierAfterStages:MTLStageAll beforeQueueStages:MTLStageAll visibilityOptions:vis];
     [e endEncoding]; g_last_end_relaxed = 0;
 }
 // DM_NVN_BARRIER=game (default full): draw passes are ordered by the game's own barriers. A draw pass ends with a barrier from its vertex and fragment stages to the fragment, compute and copy stages of
@@ -864,7 +869,7 @@ static uint64_t g_blit_splits;   // copies / buffer updates / fills that ended a
 uint64_t mtl_blit_splits(void) { return g_blit_splits; }
 static int enc_open(void);
 static int g_blit_ct;   // the open copy encoder has the compute argument table set (dispatches share it, dispatch_enc)
-static id<MTL4ComputeCommandEncoder> blit_open(void) { if (!g_blit) { ser_gate(); g_blit = enc_b_begin(g_cb); [g_blit barrierAfterQueueStages:MTLStageAll beforeStages:MTLStageAll visibilityOptions:MTL4VisibilityOptionDevice]; gp_begin(g_blit, 'C', nil); g_bw_n = g_br_n = 0; g_blit_ops = 0; g_blit_ct = 0; } return g_blit; }
+static id<MTL4ComputeCommandEncoder> blit_open(void) { if (!g_blit) { ser_gate(); g_blit = enc_b_begin(g_cb); [g_blit barrierAfterQueueStages:MTLStageAll beforeStages:MTLStageAll visibilityOptions:blit_vis()]; gp_begin(g_blit, 'C', nil); g_bw_n = g_br_n = 0; g_blit_ops = 0; g_blit_ct = 0; } return g_blit; }
 static void ubd_flush(void); static int g_nubd;   // the inline updates deferred (see mtl_sub_update)
 static int g_ubd_landing;
 static id<MTL4ComputeCommandEncoder> blit_raw(void) {
@@ -878,7 +883,7 @@ static void ubd_land_compute(void) { if (g_nubd && !g_ubd_landing) { ubd_land();
 static void blit_barrier(void) {
     static int off = -1; if (off < 0) { const char *e = getenv("DM_NVN_BLIT_ORDER"); off = e && !strcmp(e, "0"); if (off) MLOG("DM_NVN_BLIT_ORDER=0: no barriers inside the copy encoder (diagnostic; brings back the fountain's vertical stripes)"); }
     // Only the stages a compute encoder has: MTLStageAll here fails Metal validation (macOS 27: "must be a valid combination of MTLStages for the current encoder type")
-    if (!off) [g_blit barrierAfterEncoderStages:MTLStageDispatch | MTLStageBlit beforeEncoderStages:MTLStageDispatch | MTLStageBlit visibilityOptions:MTL4VisibilityOptionDevice];
+    if (!off) [g_blit barrierAfterEncoderStages:MTLStageDispatch | MTLStageBlit beforeEncoderStages:MTLStageDispatch | MTLStageBlit visibilityOptions:blit_vis()];
     g_bw_n = g_br_n = 0; g_blit_ops = 0; g_blit_barriers++;
 }
 uint64_t mtl_blit_barriers(void) { return g_blit_barriers; }
@@ -887,9 +892,16 @@ static int br_hit(uint64_t lo, uint64_t hi) { for (int i = 0; i < g_br_n; i++) i
 // buffer → buffer / fill: reads [rlo, rhi) (0, 0 when nothing is read), writes [wlo, whi)
 static uint64_t g_wr_epoch;   // Bumped by every buffer write outside a render pass (copy / update / fill, texture → buffer, dispatch): indirect-draw conversions made before it are stale (ixc_run)
 static uint64_t g_mdx_epoch;  // Bumped by GPU writes that can change an indirect-count multi-draw's records or count (copy, fill, texture → buffer, dispatch; see mdx_run)
+int nvn_sparse_alias(uint64_t lo, uint64_t hi);   // hle_nvn.c: a sparse guest range may alias another pool at a different address
 static id<MTL4ComputeCommandEncoder> blit_buf(uint64_t rlo, uint64_t rhi, uint64_t wlo, uint64_t whi) {
     g_wr_epoch++;
+    g_blit_alias = 1;   // before blit_raw, so an encoder it opens starts with alias visibility
     id<MTL4ComputeCommandEncoder> e = blit_raw();
+    g_blit_alias = 1;   // again: blit_raw may have ended an encoder in between
+    // Sparse mappings can join unrelated physical runs, so conservatively order
+    // their operations with everything before and after. Ordinary pools keep the
+    // range-based batching, including separate Metal buffers over one CPU range.
+    if (nvn_sparse_alias(rlo, rhi) || nvn_sparse_alias(wlo, whi)) { wlo = 0; whi = ~0ull; }
     if (g_blit_ops && (g_bw_n == BW_MAX || g_br_n == BW_MAX || (rhi > rlo && bw_hit(rlo, rhi)) || bw_hit(wlo, whi) || br_hit(wlo, whi))) blit_barrier();
     g_bw_lo[g_bw_n] = wlo; g_bw_hi[g_bw_n++] = whi; g_blit_ops++; g_gp_copies++;
     if (rhi > rlo) { g_br_lo[g_br_n] = rlo; g_br_hi[g_br_n++] = rhi; }

dm/gfx/tests/.gitignore

@@ -74,6 +74,8 @@ t_readback
 t_readback.dSYM/
 t_linear_readback
 t_linear_readback.dSYM/
+t_copy_alias
+t_copy_alias.dSYM/
 t_pso_pending
 t_pso_pending.dSYM/
 t_integer_blend

dm/gfx/tests/Makefile

@@ -138,6 +138,13 @@ run: check_t_upload_budget
 .PHONY: check_t_upload_budget
 t_linear_readback: t_linear_readback.m $(MTL_T_DEP)
 	clang $(CFLAGS) -fobjc-arc -o $@ t_linear_readback.m ../../host/crash.c ../../host/hle_bigstack.c t_host_stubs.c $(filter-out ../nvn/nvn_mtl.m,$(GFX)) $(FW)
+t_copy_alias: t_copy_alias.m $(MTL_T_DEP)
+	clang $(CFLAGS) -fobjc-arc -o $@ t_copy_alias.m ../../host/crash.c ../../host/hle_bigstack.c t_host_stubs.c $(filter-out ../nvn/nvn_mtl.m,$(GFX)) $(FW)
+.PHONY: check_t_copy_alias
+check_t_copy_alias: t_copy_alias
+	DM_NVN_ENC_THREAD=0 MTL_DEBUG_LAYER=1 ./t_copy_alias
+	DM_NVN_ENC_THREAD=1 MTL_DEBUG_LAYER=1 ./t_copy_alias
+run: check_t_copy_alias
 .PHONY: check_t_linear_readback
 check_t_linear_readback: t_linear_readback
 	MTL_DEBUG_LAYER=1 ./t_linear_readback

dm/gfx/tests/t_copy_alias.m

@@ -0,0 +1,89 @@
+// Copies through distinct Metal buffers and sparse guest aliases must see earlier GPU writes.
+#define nvn_game_fps nvn_game_fps_mtl_default
+#include "../nvn/nvn_mtl.m"
+#undef nvn_game_fps
+#define g_ft g_ft_hle
+#include "../nvn/hle_nvn.c"
+#undef g_ft
+uint64_t g_t0_ns;
+void *front_metal_layer(void) { return NULL; }
+module_t *module_of(const void *p) { (void)p; return NULL; }
+module_t g_mod[MAX_MOD]; int g_nmod;
+void svc_thread_dump(void) {}
+uint64_t now_ns(void) { return bp_now(); }
+static int checks, failed;
+#define CHECK(c, label) do { checks++; if (!(c)) { failed++; fprintf(stderr, "FAIL: %s\n", label); } } while (0)
+
+static void finish(void) {
+    CHECK(mtl_wait(mtl_sub_commit(), 5000000000ull), "GPU submission completed");
+    CHECK(!mtl_last_commit_error(), "GPU submission has no error");
+}
+static void bytes(const void *got, const void *want, size_t n, const char *label) {
+    CHECK(!memcmp(got, want, n), label);
+}
+static void separate_buffers(void) {
+    size_t pg = getpagesize();
+    void *mem = mmap(NULL, pg, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+    CHECK(mem != MAP_FAILED, "no-copy backing allocated"); if (mem == MAP_FAILED) abort();
+    id<MTLBuffer> a = [g_dev newBufferWithBytesNoCopy:mem length:pg options:MTLResourceStorageModeShared deallocator:nil];
+    id<MTLBuffer> b = [g_dev newBufferWithBytesNoCopy:mem length:pg options:MTLResourceStorageModeShared deallocator:nil];
+    id<MTLBuffer> io = [g_dev newBufferWithLength:pg options:MTLResourceStorageModeShared];
+    CHECK(a && b && io && a.gpuAddress != b.gpuAddress, "one allocation has distinct GPU addresses");
+    if (!a || !b || !io) abort(); res_add(a); res_add(b); res_add(io);
+    for (int split = 0; split < 2; split++) {
+        memset(mem, 0, pg); memset(io.contents, 0, pg); memset(io.contents, 0x6d, 256);
+        mtl_sub_begin();
+        enc_b_copy_buffer(blit_buf(0x10000, 0x10100, (uint64_t)mem, (uint64_t)mem + 256), io, 0, a, 0, 256);
+        if (split) { enc_b_end(g_blit); g_blit = nil; }
+        enc_b_copy_buffer(blit_buf((uint64_t)mem, (uint64_t)mem + 256, 0x20000, 0x20100), b, 0, io, 512, 256);
+        finish(); bytes((uint8_t *)io.contents + 512, io.contents, 256, "copy reads through a second no-copy buffer");
+    }
+    res_remove(a); res_remove(b); res_remove(io); a = b = nil; munmap(mem, pg);
+}
+static void sparse_copies(void) {
+    const size_t n = 256, len = 4 * getpagesize();
+    uint8_t *mem = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+    uint8_t *io = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+    CHECK(mem != MAP_FAILED && io != MAP_FAILED, "sparse test storage allocated");
+    if (mem == MAP_FAILED || io == MAP_FAILED) abort();
+    pool_t physical, sparse;
+    pool_builder_t pb = {.mem=mem, .size=len, .flags=0x24};
+    CHECK(h_nvnMemoryPoolInitialize((NVNmemoryPool *)&physical, (const NVNmemoryPoolBuilder *)&pb), "physical pool initialized");
+    pb = (pool_builder_t){.size=DM_GAME_APP_POOL + 2 * len, .flags=0x200};
+    CHECK(h_nvnMemoryPoolInitialize((NVNmemoryPool *)&sparse, (const NVNmemoryPoolBuilder *)&pb), "sparse pool initialized");
+    map_req maps[] = {{.phys=(NVNmemoryPool *)&physical, .size=len}, {.phys=(NVNmemoryPool *)&physical, .voff=len, .size=len}};
+    CHECK(h_nvnMemoryPoolMapVirtual((NVNmemoryPool *)&sparse, 2, (const NVNmappingRequest *)maps), "two sparse aliases mapped");
+    mtl_pool_add(io, len);
+    CHECK(sparse.mem != mem, "sparse pool has a distinct guest address");
+    mem[0] = 0x51; CHECK(sparse.mem[0] == 0x51 && sparse.mem[len] == 0x51, "sparse mappings share physical bytes");
+    // Both dependency directions and two virtual aliases; separate encoders also
+    // exercise the producer and consumer barriers at encoder boundaries.
+    for (int split = 0; split < 2; split++) for (int direction = 0; direction < 3; direction++) for (int op = 0; op < 4; op++) {
+        memset(mem, 0, len); memset(io, 0, len);
+        uint32_t value = op == 3 ? 0x78563412 : 0x6d6d6d6d;
+        for (size_t i = 0; i < n; i += 4) memcpy(io + i, &value, 4);
+        uint64_t dst = (uint64_t)(direction == 0 ? mem : sparse.mem);
+        uint64_t src = (uint64_t)(direction == 1 ? mem : sparse.mem + (direction == 2 ? len : 0));
+        mtl_sub_begin();
+        if (op == 0) mtl_sub_copy((uint64_t)io, dst, n);
+        else if (op == 1) mtl_sub_update(io, dst, n);
+        else mtl_sub_fill(dst, n, value);
+        if (split) ubd_land_compute();
+        if (split && g_blit) { enc_b_end(g_blit); g_blit = nil; }
+        uint64_t before = mtl_blit_barriers();
+        mtl_sub_copy(src, (uint64_t)io + 512, n);
+        if (!split) CHECK(mtl_blit_barriers() > before, "sparse alias dependency inserts an intra-encoder barrier");
+        finish(); bytes(io + 512, io, n, "sparse alias consumer reads the preceding copy/update/fill");
+    }
+    // Ordinary non-overlapping buffers keep their existing batching.
+    mtl_sub_begin(); uint64_t before = mtl_blit_barriers();
+    mtl_sub_copy((uint64_t)io, (uint64_t)mem, n);
+    mtl_sub_copy((uint64_t)io + 1024, (uint64_t)mem + 1024, n);
+    CHECK(mtl_blit_barriers() == before, "independent ordinary copies need no extra barrier"); finish();
+    h_nvnMemoryPoolFinalize((NVNmemoryPool *)&sparse); h_nvnMemoryPoolFinalize((NVNmemoryPool *)&physical); mtl_pool_del(io);
+    defer_drain(); munmap(mem, len); munmap(io, len);
+}
+int main(void) { @autoreleasepool {
+    mtl_init(); separate_buffers(); sparse_copies();
+    printf("t_copy_alias: %d checks, %d failed\n", checks, failed); return failed != 0;
+} }