From faa6b804552ee64ec1d88f14c35142554fe17f14 Mon Sep 17 00:00:00 2001 From: Chao Zhang Date: Fri, 6 Dec 2024 13:35:48 +0800 Subject: [PATCH 1/5] fix windows build issue --- backends/xnnpack/CMakeLists.txt | 38 +++++++++++----- build/resolve_buck.py | 6 +++ install_executorch.bat | 10 +---- runtime/platform/compat_unistd.h | 75 ++++++++++++++++++++++++++++++++ runtime/platform/compiler.h | 8 +--- runtime/platform/targets.bzl | 1 + setup.py | 23 +++++++--- 7 files changed, 129 insertions(+), 32 deletions(-) create mode 100644 runtime/platform/compat_unistd.h diff --git a/backends/xnnpack/CMakeLists.txt b/backends/xnnpack/CMakeLists.txt index a703d67c1b2..7f8e34b7bbf 100644 --- a/backends/xnnpack/CMakeLists.txt +++ b/backends/xnnpack/CMakeLists.txt @@ -74,17 +74,33 @@ foreach(fbs_file ${_xnnpack_schema__srcs}) endforeach() # Generate the headers from the .fbs files. -add_custom_command( - OUTPUT ${_xnnpack_schema__outputs} - COMMAND - ${FLATC_EXECUTABLE} --cpp --cpp-std c++11 --scoped-enums -o - "${_xnnpack_schema__include_dir}/executorch/backends/xnnpack/serialization" - ${_xnnpack_schema__srcs} - COMMAND mv ${_xnnpack_flatbuffer__outputs} ${_xnnpack_schema__outputs} - WORKING_DIRECTORY ${EXECUTORCH_ROOT} - COMMENT "Generating xnnpack_schema headers" - VERBATIM -) +if(WIN32) + add_custom_command( + OUTPUT ${_xnnpack_schema__outputs} + COMMAND + ${FLATC_EXECUTABLE} --cpp --cpp-std c++11 --scoped-enums -o + "${_xnnpack_schema__include_dir}/executorch/backends/xnnpack/serialization" + ${_xnnpack_schema__srcs} + COMMAND + powershell -Command + "Move-Item -Path ${_xnnpack_flatbuffer__outputs} -Destination ${_xnnpack_schema__outputs}" + WORKING_DIRECTORY ${EXECUTORCH_ROOT} + COMMENT "Generating xnnpack_schema headers" + VERBATIM + ) +else() + add_custom_command( + OUTPUT ${_xnnpack_schema__outputs} + COMMAND + ${FLATC_EXECUTABLE} --cpp --cpp-std c++11 --scoped-enums -o + "${_xnnpack_schema__include_dir}/executorch/backends/xnnpack/serialization" + ${_xnnpack_schema__srcs} + COMMAND mv ${_xnnpack_flatbuffer__outputs} ${_xnnpack_schema__outputs} + WORKING_DIRECTORY ${EXECUTORCH_ROOT} + COMMENT "Generating xnnpack_schema headers" + VERBATIM + ) +endif() add_library(xnnpack_schema INTERFACE ${_xnnpack_schema__outputs}) set_target_properties(xnnpack_schema PROPERTIES LINKER_LANGUAGE CXX) diff --git a/build/resolve_buck.py b/build/resolve_buck.py index 725a326ea67..8afcf5d3262 100644 --- a/build/resolve_buck.py +++ b/build/resolve_buck.py @@ -95,6 +95,12 @@ class BuckInfo: "c7d378f3f307e9590f0b29a5f7f1b21b8e784f4e4bd30a0160b2a69df50d2ee0" ], ), + ("windows", "x86_64"): BuckInfo( + archive_name="buck2-x86_64-pc-windows-msvc.exe.zst", + target_versions=[ + "bf1685c4c4ddd9de4592b5a955cb7326fd01e6c4d5f561643422bed961a17401" + ], + ), } diff --git a/install_executorch.bat b/install_executorch.bat index 863ade7bdbb..4cd59f2698f 100644 --- a/install_executorch.bat +++ b/install_executorch.bat @@ -7,14 +7,8 @@ rem This batch file provides a basic functionality similar to the bash script. cd /d "%~dp0" -rem Find the names of the python tools to use (replace with your actual python installation) -if "%PYTHON_EXECUTABLE%"=="" ( - if "%CONDA_DEFAULT_ENV%"=="" OR "%CONDA_DEFAULT_ENV%"=="base" OR NOT EXIST "python" ( - set PYTHON_EXECUTABLE=python3 - ) else ( - set PYTHON_EXECUTABLE=python - ) -) +rem Under windows it's always python +set PYTHON_EXECUTABLE=python "%PYTHON_EXECUTABLE%" install_executorch.py %* diff --git a/runtime/platform/compat_unistd.h b/runtime/platform/compat_unistd.h new file mode 100644 index 00000000000..c8bc4866702 --- /dev/null +++ b/runtime/platform/compat_unistd.h @@ -0,0 +1,75 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * @file + * unistd.h related macros for POSIX/Windows compatibility. + */ +#pragma once + +#if defined(_WIN32) && !defined(_WIN64) +#error \ + "You're trying to build ExecuTorch with a too old version of Windows. We need Windows 64-bit." +#endif + +#if !defined(_WIN64) +#include +#else +#include +#define O_RDONLY _O_RDONLY +#define open _open +#define close _close +#define read _read +#define write _write +#define stat _stat64 +#define fstat _fstat64 +#define off_t _off_t +#define lseek _lseeki64 + +#include // For ssize_t. +#include +// To avoid conflicts with std::numeric_limits::max() in +// file_data_loader.cpp. +#undef max + +inline ssize_t pread(int fd, void* buf, size_t nbytes, size_t offset) { + OVERLAPPED overlapped; /* The offset for ReadFile. */ + memset(&overlapped, 0, sizeof(overlapped)); + overlapped.Offset = offset; + overlapped.OffsetHigh = offset >> 32; + + BOOL result; /* The result of ReadFile. */ + DWORD bytes_read; /* The number of bytes read. */ + HANDLE file = (HANDLE)_get_osfhandle(fd); + + result = ReadFile(file, buf, nbytes, &bytes_read, &overlapped); + DWORD error = GetLastError(); + if (!result) { + if (error == ERROR_IO_PENDING) { + result = GetOverlappedResult(file, &overlapped, &bytes_read, TRUE); + if (!result) { + error = GetLastError(); + } + } + } + if (!result) { + // Translate error into errno. + switch (error) { + case ERROR_HANDLE_EOF: + errno = 0; + break; + default: + errno = EIO; + break; + } + return -1; + } + return bytes_read; +} + +#endif // !defined(_WIN64) \ No newline at end of file diff --git a/runtime/platform/compiler.h b/runtime/platform/compiler.h index 7467d5c1e04..e02324f44d5 100644 --- a/runtime/platform/compiler.h +++ b/runtime/platform/compiler.h @@ -100,14 +100,8 @@ #endif // (__cplusplus) >= 202002L /// Define a C symbol with weak linkage. -#ifdef _MSC_VER -// There currently doesn't seem to be a great way to do this in Windows and -// given that weak linkage is not really critical on Windows, we'll just leave -// it as a stub. -#define ET_WEAK -#else +// Building on Windows also need this. Windows build uses clang-cl compiler, which supports __attribute__((weak)). #define ET_WEAK __attribute__((weak)) -#endif /** * Annotation marking a function as printf-like, providing compiler support diff --git a/runtime/platform/targets.bzl b/runtime/platform/targets.bzl index 42bb851e2cf..68322ffe97f 100644 --- a/runtime/platform/targets.bzl +++ b/runtime/platform/targets.bzl @@ -68,6 +68,7 @@ def define_common_targets(): "log.h", "profiler.h", "runtime.h", + "compat_unistd.h", ], srcs = [ "abort.cpp", diff --git a/setup.py b/setup.py index 5645708a4ca..1160b40ae1c 100644 --- a/setup.py +++ b/setup.py @@ -243,7 +243,11 @@ def src_path(self, installer: "InstallerBuildExt") -> Path: src_path = src_path.replace("%BUILD_TYPE%", cfg) else: # Remove %BUILD_TYPE% from the path. +<<<<<<< HEAD src_path = src_path.replace("/%BUILD_TYPE%", "") +======= + self.src = self.src.replace("%BUILD_TYPE%/", "") +>>>>>>> 038fa5307 (fix windows build issue) # Construct the full source path, resolving globs. If there are no glob # pattern characters, this will just ensure that the source file exists. @@ -291,7 +295,7 @@ def __init__( output is in a subdirectory named after the build type. For single- config generators (like Makefile Generators or Ninja), this placeholder will be removed. - src_name: The name of the file to install + src_name: The name of the file to install. dst: The path to install to, relative to the root of the pip package. If dst ends in "/", it is treated as a directory. Otherwise it is treated as a filename. @@ -345,13 +349,17 @@ def inplace_dir(self, installer: "InstallerBuildExt") -> Path: class BuiltExtension(_BaseExtension): """An extension that installs a python extension that was built by cmake.""" - def __init__(self, src: str, modpath: str): + def __init__(self, src_dir: str, src_name: str, modpath: str): """Initializes a BuiltExtension. Args: - src: The path to the file to install (typically a shared library), - relative to the cmake-out directory. May be an fnmatch-style - glob that matches exactly one file. If the path ends in `.so`, + src_dir: The directory of the file to install, relative to the cmake-out + directory. A placeholder %BUILD_TYPE% will be replaced with the build + type for multi-config generators (like Visual Studio) where the build + output is in a subdirectory named after the build type. For single- + config generators (like Makefile Generators or Ninja), this placeholder + will be removed. + src_name: The name of the file to install. If the path ends in `.so`, this class will also look for similarly-named `.dylib` files. modpath: The dotted path of the python module that maps to the extension. @@ -359,6 +367,7 @@ def __init__(self, src: str, modpath: str): assert ( "/" not in modpath ), f"modpath must be a dotted python module path: saw '{modpath}'" + src = os.path.join(src_dir, src_name) # This is a real extension, so use the modpath as the name. super().__init__(src=f"%CMAKE_CACHE_DIR%/{src}", dst=modpath, name=modpath) @@ -783,7 +792,9 @@ def get_ext_modules() -> List[Extension]: # portable kernels, and a selection of backends. This lets users # load and execute .pte files from python. BuiltExtension( - "_portable_lib.*", "executorch.extension.pybindings._portable_lib" + src_dir="%BUILD_TYPE%/", # Set the src directory based on build configuration for windows. + src_name="_portable_lib.cp*", # Rename _portable_lib.* to _portable_lib.cp* to avoid _portable_lib.lib is selected on windows. + modpath="executorch.extension.pybindings._portable_lib", ) ) if ShouldBuild.training(): From 5c40efb6a1569d0ec94e21b551d04625989e81dd Mon Sep 17 00:00:00 2001 From: Chao Zhang Date: Fri, 6 Dec 2024 15:00:56 +0800 Subject: [PATCH 2/5] revert xnnpack cmakelist and fix file_data_loader --- backends/xnnpack/CMakeLists.txt | 38 +++++++--------------- extension/data_loader/file_data_loader.cpp | 5 ++- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/backends/xnnpack/CMakeLists.txt b/backends/xnnpack/CMakeLists.txt index 7f8e34b7bbf..a703d67c1b2 100644 --- a/backends/xnnpack/CMakeLists.txt +++ b/backends/xnnpack/CMakeLists.txt @@ -74,33 +74,17 @@ foreach(fbs_file ${_xnnpack_schema__srcs}) endforeach() # Generate the headers from the .fbs files. -if(WIN32) - add_custom_command( - OUTPUT ${_xnnpack_schema__outputs} - COMMAND - ${FLATC_EXECUTABLE} --cpp --cpp-std c++11 --scoped-enums -o - "${_xnnpack_schema__include_dir}/executorch/backends/xnnpack/serialization" - ${_xnnpack_schema__srcs} - COMMAND - powershell -Command - "Move-Item -Path ${_xnnpack_flatbuffer__outputs} -Destination ${_xnnpack_schema__outputs}" - WORKING_DIRECTORY ${EXECUTORCH_ROOT} - COMMENT "Generating xnnpack_schema headers" - VERBATIM - ) -else() - add_custom_command( - OUTPUT ${_xnnpack_schema__outputs} - COMMAND - ${FLATC_EXECUTABLE} --cpp --cpp-std c++11 --scoped-enums -o - "${_xnnpack_schema__include_dir}/executorch/backends/xnnpack/serialization" - ${_xnnpack_schema__srcs} - COMMAND mv ${_xnnpack_flatbuffer__outputs} ${_xnnpack_schema__outputs} - WORKING_DIRECTORY ${EXECUTORCH_ROOT} - COMMENT "Generating xnnpack_schema headers" - VERBATIM - ) -endif() +add_custom_command( + OUTPUT ${_xnnpack_schema__outputs} + COMMAND + ${FLATC_EXECUTABLE} --cpp --cpp-std c++11 --scoped-enums -o + "${_xnnpack_schema__include_dir}/executorch/backends/xnnpack/serialization" + ${_xnnpack_schema__srcs} + COMMAND mv ${_xnnpack_flatbuffer__outputs} ${_xnnpack_schema__outputs} + WORKING_DIRECTORY ${EXECUTORCH_ROOT} + COMMENT "Generating xnnpack_schema headers" + VERBATIM +) add_library(xnnpack_schema INTERFACE ${_xnnpack_schema__outputs}) set_target_properties(xnnpack_schema PROPERTIES LINKER_LANGUAGE CXX) diff --git a/extension/data_loader/file_data_loader.cpp b/extension/data_loader/file_data_loader.cpp index 1d097cfd989..53696cd2e4b 100644 --- a/extension/data_loader/file_data_loader.cpp +++ b/extension/data_loader/file_data_loader.cpp @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include @@ -71,6 +71,9 @@ FileDataLoader::~FileDataLoader() { std::free(const_cast(file_name_)); // fd_ can be -1 if this instance was moved from, but closing a negative fd is // safe (though it will return an error). + if (fd_ == -1) { + return; + } ::close(fd_); } From 4083ae964aedc14e2d76af9ba570759dbad53514 Mon Sep 17 00:00:00 2001 From: "Chao Zhang (from Dev Box)" Date: Thu, 12 Dec 2024 10:50:36 +0800 Subject: [PATCH 3/5] fix lint warning --- extension/data_loader/file_data_loader.cpp | 3 ++- runtime/platform/compiler.h | 3 ++- setup.py | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/extension/data_loader/file_data_loader.cpp b/extension/data_loader/file_data_loader.cpp index 53696cd2e4b..f0b01509ecf 100644 --- a/extension/data_loader/file_data_loader.cpp +++ b/extension/data_loader/file_data_loader.cpp @@ -14,10 +14,11 @@ #include #include +#include #include #include #include -#include + #include #include diff --git a/runtime/platform/compiler.h b/runtime/platform/compiler.h index e02324f44d5..d8f443f1a73 100644 --- a/runtime/platform/compiler.h +++ b/runtime/platform/compiler.h @@ -100,7 +100,8 @@ #endif // (__cplusplus) >= 202002L /// Define a C symbol with weak linkage. -// Building on Windows also need this. Windows build uses clang-cl compiler, which supports __attribute__((weak)). +// Building on Windows also need this. Windows build uses clang-cl compiler, +// which supports __attribute__((weak)). #define ET_WEAK __attribute__((weak)) /** diff --git a/setup.py b/setup.py index 1160b40ae1c..291e4b836b3 100644 --- a/setup.py +++ b/setup.py @@ -792,8 +792,8 @@ def get_ext_modules() -> List[Extension]: # portable kernels, and a selection of backends. This lets users # load and execute .pte files from python. BuiltExtension( - src_dir="%BUILD_TYPE%/", # Set the src directory based on build configuration for windows. - src_name="_portable_lib.cp*", # Rename _portable_lib.* to _portable_lib.cp* to avoid _portable_lib.lib is selected on windows. + src_dir="%BUILD_TYPE%/",# Set the src directory based on build configuration for windows. + src_name="_portable_lib.cp*",# Rename _portable_lib.* to _portable_lib.cp* to avoid _portable_lib.lib is selected on windows. modpath="executorch.extension.pybindings._portable_lib", ) ) From c77fdabfa4980f927417279587c9e22266dc1ac5 Mon Sep 17 00:00:00 2001 From: Sam Gondelman Date: Mon, 3 Mar 2025 18:06:15 -0800 Subject: [PATCH 4/5] undo non-relevant changes --- build/resolve_buck.py | 6 ------ install_executorch.bat | 10 ++++++++-- runtime/platform/compiler.h | 9 +++++++-- setup.py | 23 ++++++----------------- 4 files changed, 21 insertions(+), 27 deletions(-) diff --git a/build/resolve_buck.py b/build/resolve_buck.py index 8afcf5d3262..725a326ea67 100644 --- a/build/resolve_buck.py +++ b/build/resolve_buck.py @@ -95,12 +95,6 @@ class BuckInfo: "c7d378f3f307e9590f0b29a5f7f1b21b8e784f4e4bd30a0160b2a69df50d2ee0" ], ), - ("windows", "x86_64"): BuckInfo( - archive_name="buck2-x86_64-pc-windows-msvc.exe.zst", - target_versions=[ - "bf1685c4c4ddd9de4592b5a955cb7326fd01e6c4d5f561643422bed961a17401" - ], - ), } diff --git a/install_executorch.bat b/install_executorch.bat index 4cd59f2698f..863ade7bdbb 100644 --- a/install_executorch.bat +++ b/install_executorch.bat @@ -7,8 +7,14 @@ rem This batch file provides a basic functionality similar to the bash script. cd /d "%~dp0" -rem Under windows it's always python -set PYTHON_EXECUTABLE=python +rem Find the names of the python tools to use (replace with your actual python installation) +if "%PYTHON_EXECUTABLE%"=="" ( + if "%CONDA_DEFAULT_ENV%"=="" OR "%CONDA_DEFAULT_ENV%"=="base" OR NOT EXIST "python" ( + set PYTHON_EXECUTABLE=python3 + ) else ( + set PYTHON_EXECUTABLE=python + ) +) "%PYTHON_EXECUTABLE%" install_executorch.py %* diff --git a/runtime/platform/compiler.h b/runtime/platform/compiler.h index d8f443f1a73..7467d5c1e04 100644 --- a/runtime/platform/compiler.h +++ b/runtime/platform/compiler.h @@ -100,9 +100,14 @@ #endif // (__cplusplus) >= 202002L /// Define a C symbol with weak linkage. -// Building on Windows also need this. Windows build uses clang-cl compiler, -// which supports __attribute__((weak)). +#ifdef _MSC_VER +// There currently doesn't seem to be a great way to do this in Windows and +// given that weak linkage is not really critical on Windows, we'll just leave +// it as a stub. +#define ET_WEAK +#else #define ET_WEAK __attribute__((weak)) +#endif /** * Annotation marking a function as printf-like, providing compiler support diff --git a/setup.py b/setup.py index 291e4b836b3..5645708a4ca 100644 --- a/setup.py +++ b/setup.py @@ -243,11 +243,7 @@ def src_path(self, installer: "InstallerBuildExt") -> Path: src_path = src_path.replace("%BUILD_TYPE%", cfg) else: # Remove %BUILD_TYPE% from the path. -<<<<<<< HEAD src_path = src_path.replace("/%BUILD_TYPE%", "") -======= - self.src = self.src.replace("%BUILD_TYPE%/", "") ->>>>>>> 038fa5307 (fix windows build issue) # Construct the full source path, resolving globs. If there are no glob # pattern characters, this will just ensure that the source file exists. @@ -295,7 +291,7 @@ def __init__( output is in a subdirectory named after the build type. For single- config generators (like Makefile Generators or Ninja), this placeholder will be removed. - src_name: The name of the file to install. + src_name: The name of the file to install dst: The path to install to, relative to the root of the pip package. If dst ends in "/", it is treated as a directory. Otherwise it is treated as a filename. @@ -349,17 +345,13 @@ def inplace_dir(self, installer: "InstallerBuildExt") -> Path: class BuiltExtension(_BaseExtension): """An extension that installs a python extension that was built by cmake.""" - def __init__(self, src_dir: str, src_name: str, modpath: str): + def __init__(self, src: str, modpath: str): """Initializes a BuiltExtension. Args: - src_dir: The directory of the file to install, relative to the cmake-out - directory. A placeholder %BUILD_TYPE% will be replaced with the build - type for multi-config generators (like Visual Studio) where the build - output is in a subdirectory named after the build type. For single- - config generators (like Makefile Generators or Ninja), this placeholder - will be removed. - src_name: The name of the file to install. If the path ends in `.so`, + src: The path to the file to install (typically a shared library), + relative to the cmake-out directory. May be an fnmatch-style + glob that matches exactly one file. If the path ends in `.so`, this class will also look for similarly-named `.dylib` files. modpath: The dotted path of the python module that maps to the extension. @@ -367,7 +359,6 @@ def __init__(self, src_dir: str, src_name: str, modpath: str): assert ( "/" not in modpath ), f"modpath must be a dotted python module path: saw '{modpath}'" - src = os.path.join(src_dir, src_name) # This is a real extension, so use the modpath as the name. super().__init__(src=f"%CMAKE_CACHE_DIR%/{src}", dst=modpath, name=modpath) @@ -792,9 +783,7 @@ def get_ext_modules() -> List[Extension]: # portable kernels, and a selection of backends. This lets users # load and execute .pte files from python. BuiltExtension( - src_dir="%BUILD_TYPE%/",# Set the src directory based on build configuration for windows. - src_name="_portable_lib.cp*",# Rename _portable_lib.* to _portable_lib.cp* to avoid _portable_lib.lib is selected on windows. - modpath="executorch.extension.pybindings._portable_lib", + "_portable_lib.*", "executorch.extension.pybindings._portable_lib" ) ) if ShouldBuild.training(): From d7cbc988a3241731295f297881391e4a9ea841a6 Mon Sep 17 00:00:00 2001 From: Sam Gondelman Date: Tue, 4 Mar 2025 20:58:10 -0800 Subject: [PATCH 5/5] linter error - extra newline --- extension/data_loader/file_data_loader.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/extension/data_loader/file_data_loader.cpp b/extension/data_loader/file_data_loader.cpp index f0b01509ecf..1a9ddad259f 100644 --- a/extension/data_loader/file_data_loader.cpp +++ b/extension/data_loader/file_data_loader.cpp @@ -19,7 +19,6 @@ #include #include - #include #include #include