Re: Add ARM64 Windows port

From a6309208d0babad9abc9f836976c45c84c96d35b Mon Sep 17 00:00:00 2001
From: "SANO,Masatoshi" <[email protected]>
Date: Thu, 12 Feb 2026 15:56:08 +0900
Subject: [PATCH] Address maintainer feedback on ARM64 Windows patches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes based on review by Stas Boukarev:

1. Remove alloc tramp CSP save in tramps.lisp - alloc/alloc_list don't
   trigger GC, so saving CSP for scavenge_control_stack is unnecessary.

2. Fix reader macro style: #+sb-thread #-win32 → #+(and sb-thread (not win32))
   and #-win32 → #+(not win32) in tramps.lisp and c-call.lisp.

3. Use compile-time #ifndef LISP_FEATURE_ARM64 guard for fixedobj_size
   instead of runtime check in gencgc.c.

4. Add #if defined(LISP_FEATURE_SB_SAFEPOINT) && !defined(LISP_FEATURE_C_STACK_IS_CONTROL_STACK)
   guard around pinned objects scavenge in newspace_full_scavenge.

5. Confine call_into_c frame save/restore changes behind #ifdef LISP_FEATURE_WIN32
   in arm64-assem.S, keeping original code path untouched for other platforms.

6. Restore csel instruction in call_into_lisp for non-Windows platforms,
   guarded by #ifdef LISP_FEATURE_WIN32.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
---
 src/assembly/arm64/tramps.lisp |  9 +------
 src/compiler/arm64/c-call.lisp |  1 -
 src/runtime/arm64-assem.S      | 45 +++++++++++++++++-----------------
 src/runtime/gencgc.c           | 16 ++++++------
 4 files changed, 32 insertions(+), 39 deletions(-)

diff --git a/src/assembly/arm64/tramps.lisp b/src/assembly/arm64/tramps.lisp
index 367612be8..d548943c1 100644
--- a/src/assembly/arm64/tramps.lisp
+++ b/src/assembly/arm64/tramps.lisp
@@ -80,11 +80,6 @@
                       (inst stp cfp-tn lr-tn (@ csp-tn -112))
 
                       (map-pairs stp csp-tn -80 lisp-registers)
-                      ;; Update thread->control_stack_pointer to include the saved
-                      ;; Lisp registers so that scavenge_control_stack scans them
-                      ;; if GC is triggered by the C allocation function.
-                      #+sb-thread
-                      (storew csp-tn thread-tn thread-control-stack-pointer-slot)
                       (map-pairs stp nsp-tn 0 float-registers :pre-index -512 :delta 32)
 
                       (invoke-foreign-routine ,c-name nl3)
@@ -95,9 +90,7 @@
                       (inst ldr lr-tn (@ csp-tn -104))
 
                       (inst sub csp-tn csp-tn (+ 32 80)) ;; deallocate the frame
-                      ;; Windows uses control-stack-pointer as actual SP, not FFCA flag
-                      #+sb-thread
-                      #-win32
+                      #+(and sb-thread (not win32))
                       (inst str zr-tn (@ thread-tn (* thread-control-stack-pointer-slot n-word-bytes)))
                       #-sb-thread
                       (progn
diff --git a/src/compiler/arm64/c-call.lisp b/src/compiler/arm64/c-call.lisp
index 18b0f3b11..43b98d227 100644
--- a/src/compiler/arm64/c-call.lisp
+++ b/src/compiler/arm64/c-call.lisp
@@ -481,7 +481,6 @@
         ;; No longer OK to run GC except at safepoints.
         #+(or sb-safepoint nonstop-foreign-call)
         (storew zr-tn thread-tn thread-saved-csp-slot))
-      ;; Windows uses control-stack-pointer as actual SP, not FFCA flag
       #-win32
       (storew zr-tn thread-tn thread-control-stack-pointer-slot)
       return
diff --git a/src/runtime/arm64-assem.S b/src/runtime/arm64-assem.S
index 9181e90f0..34fd25543 100644
--- a/src/runtime/arm64-assem.S
+++ b/src/runtime/arm64-assem.S
@@ -280,9 +280,8 @@ Lno_args:
         ldr     reg_LR, [reg_LEXENV, #CLOSURE_FUN_OFFSET]
         blr     reg_LR
 
-        // NOTE: We do NOT adjust CSP here. The Lisp calling convention ensures that
-        // the callee restores CSP to its pre-call value before returning.
-        // The original code had a csel instruction here that was buggy.
+	// Correct stack pointer for return processing.
+        csel reg_CSP, reg_OCFP, reg_CSP, eq
 
         // Return value
         mov     x0, reg_R0
@@ -341,17 +340,22 @@ GNAME(call_into_c):
         // All other C arguments are already stashed on the C stack.
 
         // Build a Lisp stack frame.
-        // Save the current thread structure CFP/CSP first, then our own CFP/LR.
-#ifdef LISP_FEATURE_SB_THREAD
-	ldp     x3, x4, [reg_THREAD, THREAD_CONTROL_FRAME_POINTER_OFFSET]  // Load thread CFP/CSP
-	stp     x3, x4, [reg_CSP, #-16]!  // Push thread CFP/CSP onto stack, CSP -= 16
+#ifdef LISP_FEATURE_WIN32
+        // On Windows ARM64, save thread CFP/CSP before overwriting.
+        // CSP cannot be zeroed to indicate "in Lisp" state on safepoint builds.
+        ldp     x3, x4, [reg_THREAD, THREAD_CONTROL_FRAME_POINTER_OFFSET]
+        stp     x3, x4, [reg_CSP, #-16]!
+        stp     reg_CFP, reg_LR, [reg_CSP, #-16]!
+        add     reg_R10, reg_CSP, #32
+#else
+        // Can store two values above the stack pointer, interrupts ignore them.
+        stp     reg_CFP, reg_LR, [reg_CSP]
+        add     reg_R10, reg_CSP, #2*8
 #endif
-        stp     reg_CFP, reg_LR, [reg_CSP, #-16]!  // Push our CFP/LR, CSP -= 16
-        add     reg_R10, reg_CSP, #32  // R10 = original CSP (before both pushes)
         mov     reg_LEXENV, reg_LR
 
+        // Save the lisp stack and frame pointers.
 #ifdef LISP_FEATURE_SB_THREAD
-        // Now save our own stack pointers to thread structure.
         stp     reg_CSP, reg_R10, [reg_THREAD, THREAD_CONTROL_FRAME_POINTER_OFFSET]
 #else
         ENTER_PA
@@ -406,24 +410,21 @@ GNAME(call_into_c):
 #endif
 
 
-        // Restore the Lisp stack and frame pointers from the stack
-	// Stack layout: [CSP] = CFP, [CSP+8] = LR, [CSP+16] = thread CFP, [CSP+24] = thread CSP
-	ldp     reg_CFP, reg_LR, [reg_CSP], #16  // Restore CFP/LR, CSP += 16
-
+        // Restore the Lisp stack and frame pointers
+#ifdef LISP_FEATURE_WIN32
+        // Restore CFP/LR from stack, then restore saved thread CFP/CSP.
+        ldp     reg_CFP, reg_LR, [reg_CSP], #16
+        ldp     x3, x4, [reg_CSP], #16
+        stp     x3, x4, [reg_THREAD, THREAD_CONTROL_FRAME_POINTER_OFFSET]
+#else
 #ifdef LISP_FEATURE_SB_THREAD
-	// Now restore the thread structure CFP/CSP that we saved at entry.
-	ldp     x3, x4, [reg_CSP], #16  // Load saved thread CFP/CSP, CSP += 16
-	stp     x3, x4, [reg_THREAD, THREAD_CONTROL_FRAME_POINTER_OFFSET]  // Restore to thread structure
-
-// Windows uses control_stack_pointer as actual SP, not FFCA flag
-#ifndef LISP_FEATURE_WIN32
         str     xzr, [reg_THREAD, THREAD_CONTROL_STACK_POINTER_OFFSET]
-#endif
 #else
 	// Clear FFCA, so the runtime knows that we're "in lisp".
 	str     xzr, [reg_OCFP]
 #endif
-        mov     reg_LEXENV, reg_LR
+        mov     reg_LR, reg_LEXENV
+#endif
         ret
 
 	SIZE(call_into_c)
diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c
index 6d9c66a20..e8499d454 100644
--- a/src/runtime/gencgc.c
+++ b/src/runtime/gencgc.c
@@ -2665,6 +2665,7 @@ static void newspace_full_scavenge(generation_index_t generation)
     /* Enable recording of all new allocation regions */
     record_new_regions_below = 1 + page_table_pages;
 
+#if defined(LISP_FEATURE_SB_SAFEPOINT) && !defined(LISP_FEATURE_C_STACK_IS_CONTROL_STACK)
     /* Scavenge pinned from_space objects. These objects reside on pages with
      * gen=from_space, so they are NOT processed by scavenge_root_gens (which
      * requires gen >= from) or the newspace scan above (which requires
@@ -2694,6 +2695,7 @@ static void newspace_full_scavenge(generation_index_t generation)
             }
         }
     }
+#endif
 }
 
 void gc_close_collector_regions(int flag)
@@ -3949,14 +3951,12 @@ collect_garbage(generation_index_t last_gen)
 #ifdef LISP_FEATURE_IMMOBILE_SPACE
   if (ENABLE_PAGE_PROTECTION) {
       // Unprotect the in-use ranges. Any page could be written during scavenge
-      // On some platforms (e.g., ARM64), fixedobj space may be disabled
-      // (FIXEDOBJ_SPACE_START=0, size=0), so skip the protection change.
-      uword_t fixedobj_size = (lispobj)fixedobj_free_pointer - FIXEDOBJ_SPACE_START;
-      if (FIXEDOBJ_SPACE_START != 0 && fixedobj_size > 0) {
-          os_protect((os_vm_address_t)FIXEDOBJ_SPACE_START,
-                     fixedobj_size,
-                     OS_VM_PROT_ALL);
-      }
+#ifndef LISP_FEATURE_ARM64
+      // ARM64 does not use fixedobj space (FIXEDOBJ_SPACE_START=0).
+      os_protect((os_vm_address_t)FIXEDOBJ_SPACE_START,
+                 (lispobj)fixedobj_free_pointer - FIXEDOBJ_SPACE_START,
+                 OS_VM_PROT_ALL);
+#endif
   }
 #endif
 
-- 
2.43.0