Skip to content

Conversation

@jgarzik
Copy link
Owner

@jgarzik jgarzik commented Jan 15, 2026

Implement Windows x64 ABI support with trait-based abstraction:

  • Add src/abi.rs with Abi trait, SysV64 and Win64 implementations
  • Refactor codegen.rs to use PlatformAbi for register selection
  • Reorganize runtime: move *.s to runtime/sysv/, add runtime/win64/
  • Win64 runtime handles: different arg registers (rcx,rdx,r8,r9), 32-byte shadow space, varargs float in both xmm and int reg
  • Update main.rs to use gcc on Windows (MinGW)
  • Add Windows CI job with MSYS2/MINGW64

Platform selection is compile-time via #[cfg(windows)].

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Windows x64 (Win64) ABI support to the BASIC compiler, enabling cross-platform compilation on Windows, Linux, and macOS. The implementation uses a trait-based abstraction to handle ABI differences at compile time.

Changes:

  • Introduces ABI abstraction layer with trait-based platform selection
  • Adds complete Win64 runtime library with correct calling conventions
  • Reorganizes runtime files into platform-specific directories (sysv/ and win64/)

Reviewed changes

Copilot reviewed 12 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/abi.rs New ABI abstraction trait with SysV64 and Win64 implementations
src/codegen.rs Updated to use PlatformAbi for register selection
src/runtime.rs Modified to conditionally include platform-specific runtime files
src/main.rs Updated to use gcc on Windows (MinGW) instead of cc
src/runtime/win64/*.s New Win64 ABI runtime implementations (string, print, math, input, file, data)
src/runtime/sysv/*.s Relocated SysV runtime files with updated documentation
.github/workflows/TestingCI.yml Added Windows CI job with MSYS2/MINGW64
Comments suppressed due to low confidence (3)

src/codegen.rs:10

  • This comment is outdated. The compiler now supports both System V AMD64 ABI (Linux, macOS, BSD) and Win64 ABI (Windows), selected at compile time via #[cfg(windows)]. The comment should be updated to reflect this multi-platform support.
//! The generated code follows the System V AMD64 ABI for compatibility with libc functions.
//! Output is assembled with the system assembler (`as`) and linked with `cc`.

src/codegen.rs:112

  • This documentation section describes only System V AMD64 calling conventions. Since the compiler now supports both System V and Win64 ABIs, this section should either: (1) be updated to document both ABIs, or (2) be modified to note that calling conventions are platform-specific and reference the abi.rs module for details.
//! # Calling Convention (System V AMD64)
//!
//! For calling libc and runtime functions:
//!
//! | Argument # | Integer/Pointer | Float      |
//! |------------|-----------------|------------|
//! | 1          | `rdi`           | `xmm0`     |
//! | 2          | `rsi`           | `xmm1`     |
//! | 3          | `rdx`           | `xmm2`     |
//! | 4          | `rcx`           | `xmm3`     |
//! | 5          | `r8`            | `xmm4`     |
//! | 6          | `r9`            | `xmm5`     |
//!
//! Return values: integers in `rax`, floats in `xmm0`.
//!
//! **Caller-saved** (may be clobbered by calls): `rax`, `rcx`, `rdx`, `rsi`, `rdi`,
//! `r8`-`r11`, `xmm0`-`xmm15`
//!
//! **Callee-saved** (preserved across calls): `rbx`, `rbp`, `r12`-`r15`

src/codegen.rs:61

  • This comment mentions only System V AMD64 ABI. Win64 also requires 16-byte stack alignment, so the documentation is still mostly accurate, but it should acknowledge that both ABIs have this requirement. Additionally, Win64 requires 32-byte shadow space before calls, which is not mentioned here or handled in the code generation (the stack allocation only accounts for local variables, not shadow space).
//! # Stack Alignment (Critical for ABI Compliance)
//!
//! The System V AMD64 ABI requires 16-byte stack alignment before `call` instructions.
//! We maintain this invariant:
//!
//! 1. **Function entry**: After `call` pushes return address, `rsp % 16 == 8`
//! 2. **After `push rbp`**: `rsp % 16 == 0`
//! 3. **Prologue `sub rsp, N`**: N is rounded up to multiple of 16
//! 4. **Temporaries**: We use `sub rsp, 16` / `add rsp, 16` (never 8-byte pushes for temps)
//!
//! This ensures the stack is always 16-byte aligned before any `call` instruction,
//! which is required for SSE operations and varargs functions like `printf`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/abi.rs Outdated

/// Generate setup code for vararg float arguments (e.g., printf)
/// System V: just use xmm registers
/// Win64: must also copy float to corresponding integer register
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vararg_float_setup method is defined in the Abi trait but is never used in the codebase. The vararg handling is done internally within the runtime .s files rather than in codegen.rs. Consider removing this method if it's not planned for future use, or add a comment explaining its intended purpose.

Suggested change
/// Win64: must also copy float to corresponding integer register
/// Win64: must also copy float to corresponding integer register
///
/// Note: At present, vararg handling is implemented in the runtime
/// assembly (.s) files rather than in Rust `codegen.rs`, so this hook
/// is not invoked by the current Rust backend. It is kept here to
/// document the ABI contract and as a potential extension point for
/// future backends or codegen strategies. Implementations that do not
/// need extra setup should simply return an empty `Vec`.

Copilot uses AI. Check for mistakes.
src/abi.rs Outdated
impl Abi for Win64 {
const INT_ARG_REGS: &'static [&'static str] = &["rcx", "rdx", "r8", "r9"];
const FLOAT_ARG_REGS: &'static [&'static str] = &["xmm0", "xmm1", "xmm2", "xmm3"];
const SHADOW_SPACE: i32 = 32;
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SHADOW_SPACE constant is defined but never used in codegen.rs. On Win64, the caller must allocate 32 bytes of shadow space before every call instruction, but the current code generation doesn't do this. The function prologue only allocates space for local variables (-self.stack_offset), not shadow space. This could lead to stack corruption on Windows when the generated BASIC code calls runtime functions. The stack allocation logic in codegen.rs should be updated to include shadow space on Win64: let stack_needed = max(PlatformAbi::SHADOW_SPACE as i32, -self.stack_offset).

Copilot uses AI. Check for mistakes.
# rdx = haystack length
# r8 = needle pointer
# r9 = needle length
# [rsp+40] = start position (1-based) - 5th arg on stack after shadow space
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment could be clearer. It should specify that [rsp+40] refers to the caller's rsp value before the call instruction, not the rsp value inside the function. The actual access in the function uses [rbp + 48], which is correct. Consider rephrasing to: '5th argument at caller's [rsp+40] (accessed as [rbp+48] in function)' to avoid confusion.

Suggested change
# [rsp+40] = start position (1-based) - 5th arg on stack after shadow space
# 5th argument at caller's [rsp+40] (accessed as [rbp+48] in function) = start position (1-based)

Copilot uses AI. Check for mistakes.
Replace MinGW/MSYS2 with pure Win32 API runtime and MSVC toolchain.

Build changes:
- Use clang for assembly, link.exe with UCRT for linking
- CI uses ilammy/msvc-dev-cmd instead of MSYS2

New runtime (src/runtime/win64-native/):
- print.s: Console output via GetStdHandle/WriteFile
- input.s: Console input via ReadFile
- file.s: File I/O via CreateFileA/WriteFile/ReadFile
- string.s: String ops with HeapAlloc/GetProcessHeap
- math.s: RND (xorshift), TIMER (GetTickCount64), CLS
- data.s: DATA/READ/RESTORE support
- data_defs.s: Shared format strings

Codegen fixes:
- Fix register order in gen_print_expr_to_file for Win64 ABI
- Add named constants for stack sizes and character codes

All 191 tests passing on Windows.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jgarzik jgarzik merged commit 4966261 into master Jan 16, 2026
4 checks passed
@jgarzik jgarzik deleted the updates branch January 16, 2026 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant