-
Notifications
You must be signed in to change notification settings - Fork 411
Enable support for Emscripten in CMake #7859
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.
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
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
@@ -89,6 +89,15 @@ ELSEIF(CMAKE_GENERATOR MATCHES "^Visual Studio " AND CMAKE_GENERATOR_PLATFORM) | |||
ELSE() | |||
MESSAGE(FATAL_ERROR "Unsupported Visual Studio architecture \"${CMAKE_GENERATOR_PLATFORM}\"") | |||
ENDIF() | |||
ELSEIF(CMAKE_SYSTEM_NAME STREQUAL "Emscripten") | |||
SET(XNNPACK_TARGET_PROCESSOR "wasm") | |||
SET(XNNPACK_ENABLE_RISCV_VECTOR OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RISCV in particular?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the XNNPACK_ENABLE_RISCV_VECTOR
option defaults to ON, which causes the define XNN_ENABLE_RISCV_VECTOR
to be set, which causes this line:
XNNPACK/src/configs/transpose-config.c
Line 203 in 4b857f3
if (hardware_config->vlenb >= 128) { |
XNNPACK/src/configs/transpose-config.c:203:28: error: no member named 'vlenb' in 'struct xnn_hardware_config'
203 | if (hardware_config->vlenb >= 128) {
| ~~~~~~~~~~~~~~~ ^
XNNPACK/src/configs/transpose-config.c:205:61: error: use of undeclared identifier 'xnn_x32_transposec_ukernel__32x8_rvv'
205 | .const_size_ukernel = (xnn_transposec_ukernel_fn) xnn_x32_transposec_ukernel__32x8_rvv,
| ^
XNNPACK/src/configs/transpose-config.c:208:35: error: no member named 'vlenb' in 'struct xnn_hardware_config'
208 | } else if (hardware_config->vlenb == 64) {
| ~~~~~~~~~~~~~~~ ^
XNNPACK/src/configs/transpose-config.c:210:61: error: use of undeclared identifier 'xnn_x32_transposec_ukernel__16x8_rvv'
210 | .const_size_ukernel = (xnn_transposec_ukernel_fn) xnn_x32_transposec_ukernel__16x8_rvv,
| ^
XNNPACK/src/configs/transpose-config.c:213:35: error: no member named 'vlenb' in 'struct xnn_hardware_config'
213 | } else if (hardware_config->vlenb == 32) {
| ~~~~~~~~~~~~~~~ ^
XNNPACK/src/configs/transpose-config.c:215:61: error: use of undeclared identifier 'xnn_x32_transposec_ukernel__8x8_rvv'
215 | .const_size_ukernel = (xnn_transposec_ukernel_fn) xnn_x32_transposec_ukernel__8x8_rvv,
| ^
XNNPACK/src/configs/transpose-config.c:218:35: error: no member named 'vlenb' in 'struct xnn_hardware_config'
218 | } else if (hardware_config->vlenb == 16) {
| ~~~~~~~~~~~~~~~ ^
XNNPACK/src/configs/transpose-config.c:220:61: error: use of undeclared identifier 'xnn_x32_transposec_ukernel__4x4_rvv'
220 | .const_size_ukernel = (xnn_transposec_ukernel_fn) xnn_x32_transposec_ukernel__4x4_rvv,
There's possible alternative fixes (e.g. fixing the ifdefs in transpose-config.c so that code isn't compiled, or defaulting the XNNPACK_ENABLE_RISCV_VECTOR
to OFF except for RISCV builds, etc), but switching it off here in the Emscripten section was just the lowest risk of affecting anything else option I could see.
Addresses #3455
Takes the approach of detecting the use of Wasm SIMD by detecting if the requisite flags have been added to CMAKE_C_FLAGS or CMAKE_CXX_FLAGS. Adding the SIMD flags to these variables is the right way to enable Wasm SIMD globally for CMake builds, so this should do the right thing.
The use of
SET(XNNPACK_ENABLE_RISCV_VECTOR OFF)
is to suppress a compile error; without this RISC-specific code is compiled for Wasm; turning this option off here seemed the minimally invasive solution.