Skip to content

Enable fast-interpreter SIMD tests on CI #4125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 7, 2025
Merged

Enable fast-interpreter SIMD tests on CI #4125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions .github/workflows/compilation_on_android_ubuntu.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ jobs:
"-DWAMR_BUILD_PERF_PROFILING=1",
"-DWAMR_BUILD_REF_TYPES=1",
"-DWAMR_BUILD_SIMD=1",
"-DWAMR_BUILD_LIB_SIMDE=1",
"-DWAMR_BUILD_TAIL_CALL=1",
"-DWAMR_DISABLE_HW_BOUND_CHECK=1",
"-DWAMR_BUILD_MEMORY64=1",
Expand All @@ -178,11 +179,9 @@ jobs:
make_options_feature: "-DWAMR_BUILD_MULTI_MODULE=1"
- make_options_run_mode: $MULTI_TIER_JIT_BUILD_OPTIONS
make_options_feature: "-DWAMR_BUILD_MULTI_MODULE=1"
# SIMD only on JIT/AOT mode
# SIMD only on JIT/AOT/fast interpreter mode
- make_options_run_mode: $CLASSIC_INTERP_BUILD_OPTIONS
make_options_feature: "-DWAMR_BUILD_SIMD=1"
- make_options_run_mode: $FAST_INTERP_BUILD_OPTIONS
make_options_feature: "-DWAMR_BUILD_SIMD=1"
# DEBUG_INTERP only on CLASSIC INTERP mode
- make_options_run_mode: $AOT_BUILD_OPTIONS
make_options_feature: "-DWAMR_BUILD_DEBUG_INTERP=1"
Expand Down Expand Up @@ -649,11 +648,9 @@ jobs:
test_option: $WAMR_COMPILER_TEST_OPTIONS
exclude:
# incompatible modes and features
# classic-interp and fast-interp don't support simd
# classic-interp doesn't support simd
- running_mode: "classic-interp"
test_option: $SIMD_TEST_OPTIONS
- running_mode: "fast-interp"
test_option: $SIMD_TEST_OPTIONS
# llvm jit doesn't support multi module
- running_mode: "jit"
test_option: $MULTI_MODULES_TEST_OPTIONS
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/compilation_on_sgx.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ jobs:
"-DWAMR_BUILD_PERF_PROFILING=1",
"-DWAMR_BUILD_REF_TYPES=1",
# doesn't support
# "-DWAMR_BUILD_SIMD=1",
"-DWAMR_BUILD_SIMD=0",
Copy link
Contributor Author

@jammar1 jammar1 Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this was disabled anyway, just making it explicit. When I tried enabling this, SIMDe won't compile because of missing SIMD intrinsics in the SGX toolchain.

/home/runner/work/wasm-micro-runtime/wasm-micro-runtime/product-mini/platforms/linux-sgx/build/_deps/simde-src/simde/wasm/../simde-features.h:319:12: fatal error: emmintrin.h: No such file or directory
  319 |   #include <emmintrin.h>
      |            ^~~~~~~~~~~~~
compilation terminated.

"-DWAMR_BUILD_TAIL_CALL=1",
"-DWAMR_DISABLE_HW_BOUND_CHECK=1",
"-DWAMR_BUILD_SGX_IPFS=1",
Expand Down
4 changes: 2 additions & 2 deletions build-scripts/runtime_lib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ if (WAMR_BUILD_LIB_RATS EQUAL 1)
endif ()

if (WAMR_BUILD_SIMD EQUAL 1 AND WAMR_BUILD_FAST_INTERP EQUAL 1)
if (NOT (WAMR_BUILD_TARGET MATCHES "AARCH64.*" OR WAMR_BUILD_TARGET MATCHES "ARM.*"))
message(STATUS "SIMDe doesnt support platform " ${WAMR_BUILD_TARGET})
if (WAMR_BUILD_PLATFORM STREQUAL "windows")
Copy link
Contributor Author

@jammar1 jammar1 Mar 3, 2025

Choose a reason for hiding this comment

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

I plan to address this later when I can find a Windows computer. Compilation on Visual Studio is failing but this PR strictly improves the test coverage.

Copy link
Collaborator

@lum1n0us lum1n0us Mar 3, 2025

Choose a reason for hiding this comment

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

if (NOT (WAMR_BUILD_TARGET MATCHES "AARCH64.*" OR WAMR_BUILD_TARGET MATCHES "ARM.*")) does not appear to be equivalent to if (WAMR_BUILD_PLATFORM STREQUAL "windows"). Therefore, we might need to merge the two to ensure compatibility.

Copy link
Contributor Author

@jammar1 jammar1 Mar 3, 2025

Choose a reason for hiding this comment

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

They're not equivalent. I originally removed the check entirely, but I encountered some MSVC build issues.

I'm setting up a Windows computer to take a look at the issue this week, but SIMDe does support various Arm ISAs / x86 / RISC-V so this change is more correct than the existing check.

Copy link
Contributor Author

@jammar1 jammar1 Mar 5, 2025

Choose a reason for hiding this comment

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

@lum1n0us Is this okay?

Copy link
Contributor Author

@jammar1 jammar1 Mar 6, 2025

Choose a reason for hiding this comment

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

Just to clarify, the branch and tests are running and passing on x86 so this check is no longer needed.

There are build issues with Windows in CI (although it builds and runs for me locally), so I changed the check to represent that.

message(STATUS "SIMDe doesnt support platform " ${WAMR_BUILD_PLATFORM})
Copy link
Collaborator

@loganek loganek Mar 3, 2025

Choose a reason for hiding this comment

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

I wonder if that should be error instead of just a status? At that stage users set WAMR_BUILD_SIMD so they expect to have it; in case it's not supported, maybe we should notify the users more explicitly about it?

set(WAMR_BUILD_SIMDE 0)
else()
include (${IWASM_DIR}/libraries/simde/simde.cmake)
Expand Down
4 changes: 2 additions & 2 deletions tests/wamr-test-suites/test_wamr.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,8 +913,8 @@ function do_execute_in_running_mode()
fi

if [[ ${ENABLE_SIMD} -eq 1 ]]; then
if [[ "${RUNNING_MODE}" != "jit" && "${RUNNING_MODE}" != "aot" ]]; then
echo "support simd in llvm-jit mode and aot mode"
if [[ "${RUNNING_MODE}" != "jit" && "${RUNNING_MODE}" != "aot" && "${RUNNING_MODE}" != "fast-interp" ]]; then
echo "support simd in llvm-jit, aot and fast-interp mode"
return 0;
fi
fi
Expand Down
Loading