[Canary] build: wasm: Enabling wasm arm64 windows #10879
Conversation
cosmo0920
requested review from
niedbalski,
patrick-stephens,
celalettin1286,
fujimotos,
edsiper and
koleini
as code owners
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughEnables Windows ARM64 (AArch64) WAMR/WASM support: updates GitHub Actions to pass WAMR target for Windows Arm64; switches CMake to enable WASM on Windows arm64; adds ARM64 MASM assembly implementations (non-SIMD and SIMD) and configures MSVC/MASM build rules in iwasm_common.cmake. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant CMake as CMake (configure)
participant MSVC as MSVC+MASM
participant WAMR as WAMR Build
GH->>CMake: Configure with -DWAMR_BUILD_TARGET=AARCH64 (Windows Arm64)
CMake->>CMake: Detect Windows + MSVC + AArch64
alt VCToolsInstallDir present
CMake->>MSVC: Set ASM_MASM compiler/commands
CMake->>WAMR: Use armarm64.asm (+_simd.asm) sources
else Missing tools
CMake-->>GH: Fatal error (stop)
end
MSVC->>WAMR: Compile MASM sources
WAMR-->>GH: Build artifacts
sequenceDiagram
autonumber
participant VM as WASM VM (WAMR)
participant IN as invokeNative (ARM64)
participant FP as Target Func
VM->>IN: invokeNative(func_ptr, argv, nstacks)
IN->>IN: Load d0–d7, x0–x7 from argv
alt nstacks > 0
IN->>IN: Allocate aligned stack space
IN->>IN: Copy stack args from argv
end
IN->>FP: blr func_ptr
FP-->>IN: return
IN-->>VM: return
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cosmo0920
changed the title
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/iwasm_common.cmake (2)
15-25: Duplicate MASM per-file properties; consolidate to one place.You set ASM_MASM properties for the same sources both here and again within the AARCH64 branch below. Keep a single definition to avoid drift.
- Keep the top-level foreach (_WAMR_ARM64_MASM_SOURCES) block and remove the later per-file property blocks in the AARCH64 section.
- set(_WAMR_ARM64_MASM_SOURCES ${IWASM_COMMON_DIR}/arch/invokeNative_armarm64.asm) - set_source_files_properties(${_WAMR_ARM64_MASM_SOURCES} - PROPERTIES - LANGUAGE ASM_MASM - COMPILE_DEFINITIONS "" - INCLUDE_DIRECTORIES "" - COMPILE_OPTIONS "/nologo" - ) + # (properties already set at top-level for ARM64 MASM sources)- set(_WAMR_ARM64_MASM_SOURCES_SIMD ${IWASM_COMMON_DIR}/arch/invokeNative_armarm64_simd.asm) - set_source_files_properties(${_WAMR_ARM64_MASM_SOURCES_SIMD} - PROPERTIES - LANGUAGE ASM_MASM - COMPILE_DEFINITIONS "" - INCLUDE_DIRECTORIES "" - COMPILE_OPTIONS "/nologo" - ) + # (properties already set at top-level for ARM64 MASM sources)Also applies to: 107-116, 123-132
27-28: Improve failure message for missing VCToolsInstallDir.Suggest hinting to use the Developer Command Prompt action (already used in CI) and to install “MSVC v143 ARM64 build tools”.
- message(FATAL_ERROR "VCToolsInstallDir is not defined. Please run from a Developer Command Prompt or specify armasm64.exe manually.") + message(FATAL_ERROR "VCToolsInstallDir not set. Run CMake from a Developer Command Prompt (MSVC) and ensure 'MSVC ARM64 build tools' are installed.")lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64.asm (1)
25-42: Stack-args copy loop: OK; minor micro‑nit.Logic is correct. Micro‑nit: prefer cbz/cbnz for zero checks to shave one instruction, if desired.
- cmp x21, #0 - b.eq call_func + cbz x21, call_funclib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64_simd.asm (1)
35-41: Same cbz/cbnz micro‑nit for the copy loop.Optional.
- cmp x21, #0 - b.eq call_func + cbz x21, call_func
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/call-build-windows.yaml(1 hunks)cmake/windows-setup.cmake(2 hunks)lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64.asm(1 hunks)lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64_simd.asm(1 hunks)lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/iwasm_common.cmake(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
PR: fluent/fluent-bit#9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
cmake/windows-setup.cmake
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (7)
.github/workflows/call-build-windows.yaml (1)
95-95: Good: WAMR AArch64 target wired into the Arm64 Windows matrix.This matches iwasm_common.cmake’s AARCH64 branch selection and unblocks the MASM-based invokeNative. No issues spotted.
Please confirm the CMake configure log shows:
- WAMR_BUILD_PLATFORM=windows
- WAMR_BUILD_TARGET=AARCH64
- WAMR_BUILD_SIMD as intended (0/1)
cmake/windows-setup.cmake (2)
39-44: Enable FLB_WASM on Windows/arm64: OK.This aligns Windows arm64 defaults with x64/x86. Keep an eye on package size and CI time impact.
135-140: Enable FLB_FILTER_WASM on Windows/arm64: OK.Consistent with FLB_WASM above; expect new plugin to build under arm64 toolchain.
Please run a local arm64 Windows build to ensure filter_wasm links successfully with WAMR (static CRT and vcpkg triplet arm64-windows-static).
lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/iwasm_common.cmake (1)
103-120: Source selection for AARCH64 on Windows/MSVC: looks correct after fix.Once the compiler path issue is addressed, the conditional picks .asm for non‑SIMD and the SIMD variant when enabled. No other concerns.
Confirm both configurations compile:
- WAMR_BUILD_SIMD=0 (default)
- WAMR_BUILD_SIMD=1 (SIMD path)
Also applies to: 121-136
lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64.asm (2)
56-67: Argument loading order matches WAMR AArch64 ABI usage.Loading d0–d7 then x0–x7 from argv is consistent with WAMR’s calling convention expectations.
45-73: ```shell
#!/bin/bash
set -euo pipefailecho "1) Locate asm file(s) matching invokeNative* (show paths)"
rg -n --hidden --no-ignore "invokeNative[_A-Za-z0-9]*arm" -S || true
echoASM="lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64.asm"
echo "2) If the expected file exists, print the first 200 lines"
if [ -f "$ASM" ]; then
echo "=== Found: $ASM ==="
sed -n '1,200p' "$ASM"
else
echo "=== $ASM not found at expected path ==="
rg -n --hidden --no-ignore "invokeNative_armarm64.asm|invokeNative_armarm64" -S || true
fi
echoecho "3) Search the repo for unwind-related assembler directives (.cfi, .seh, fnstart/fnend, .xdata/.pdata)"
rg -n --hidden --no-ignore '.cfi_|.cfi\b|.seh|fnstart|fnend|.xdata|.pdata' -g '**/*.{s,S,asm}' -S || true
echoecho "4) Show any arm64-targeted assembly files (paths only)"
rg -n --hidden --no-ignore 'aarch64|arm64' -g '**/*.{s,S,asm}' -S || true
echoecho "5) Search build system / platform macros for Windows/Win32 references"
rg -n --hidden --no-ignore 'CMAKE_SYSTEM_NAME.*Windows|_WIN32\b|WIN32\b|MINGW|MSYS|_MSC_VER|MSVC|Windows' -S || true
echoecho "6) Search for references to this asm file in build scripts (Makefiles, CMakeLists, etc.)"
rg -n --hidden --no-ignore 'invokeNative_armarm64.asm|invokeNative_armarm64' -S || true
echoecho "7) Show top-level README/CMake/BUILD snippets (if present) to infer supported platforms"
for f in README.md README.rst CMakeLists.txt BUILD.md configure; do
if [ -f "$f" ]; then
echo "=== $f (first 200 lines) ==="
sed -n '1,200p' "$f"
echo
fi
done</blockquote></details> <details> <summary>lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64_simd.asm (1)</summary><blockquote> `56-61`: **Neon loads: efficient; keep as-is.** Using ld1 on v0–v7 is compact and correct for the argv layout. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/arch/invokeNative_armarm64_simd.asm
Show resolved
Hide resolved
lib/wasm-micro-runtime-WAMR-2.4.1/core/iwasm/common/iwasm_common.cmake
Outdated
Show resolved
Hide resolved
cosmo0920
force-pushed
the
cosmo0920-canary-enabling-wasm-arm64-windows
branch
from
140d4ab to
b91b3c7
Compare
Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
force-pushed
the
cosmo0920-canary-enabling-wasm-arm64-windows
branch
from
b58915d to
a8d5d79
Compare
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Chores