When crosscompiling, build host tools externally. #5170
Conversation
|
CMake failures are trunk failures. Make failures are because I haven't updated the Makefile. |
|
Alternate thought: since we now require Python for the python bindings and some apps anyway, maybe it's time to consider requiring it for building in general; both binary2cpp and build_halide_h would be trivial to write in Python. |
alexreinking
force-pushed
the
cmake/crosscompile
branch
2 times, most recently
from
927f7ab to
97fcbeb
Compare
| ## | ||
|
|
||
| if (CMAKE_CROSSCOMPILING) | ||
| # Note: using CMAKE_EXECUTABLE_SUFFIX breaks cross-OS builds involving Windows. |
There was a problem hiding this comment.
I'm a little confused by this comment -- it breaks the builds but we use it anyway?
There was a problem hiding this comment.
It doesn't break any existing scenarios, it just isn't a supported scenario without going full superbuild. I can make this clearer.
| if (CMAKE_CROSSCOMPILING) | ||
| # Note: using CMAKE_EXECUTABLE_SUFFIX breaks cross-OS builds involving Windows. | ||
| include(ExternalProject) | ||
| ExternalProject_Add(host-tools |
There was a problem hiding this comment.
super-duper-nit: I find it hard to quickly read the keywords-vs-arg-values here; I wonder if it would be more obvious with indentation like:
ExternalProject_Add(host-tools
BUILD_BYPRODUCTS
"<INSTALL_DIR>/bin/build_halide_h${CMAKE_EXECUTABLE_SUFFIX}"
"<INSTALL_DIR>/bin/binary2cpp${CMAKE_EXECUTABLE_SUFFIX}"
SOURCE_DIR
${CMAKE_CURRENT_LIST_DIR}/tools
INSTALL_DIR
${CMAKE_CURRENT_BINARY_DIR}/host-tools
CMAKE_ARGS
-D CMAKE_TOOLCHAIN_FILE=${Halide_HOST_TOOLCHAIN_FILE}
-D CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-D CMAKE_INSTALL_PREFIX=<INSTALL_DIR>
BUILD_ALWAYS
YES)
There was a problem hiding this comment.
The formatter in CLion is not very smart and it tramples such indentation 😞 I could add some blank lines to make it a bit more readable.
There was a problem hiding this comment.
Didn't know there existed an autoformatter for CMake. Does it exist anywhere other than CLion?
There was a problem hiding this comment.
Not that I'm aware of. There are two open-source CMake formatters I'm aware of. I've tried both. One, cmake-format does a bad job, the linter is even worse. The other, gersemi, is not very configurable, is in pre-alpha state and led to a huge diff.
| set_target_properties(binary2cpp | ||
| PROPERTIES | ||
| IMPORTED_LOCATION "${INSTALL_DIR}/bin/binary2cpp${CMAKE_EXECUTABLE_SUFFIX}" | ||
| CROSSCOMPILING_EMULATOR "") |
There was a problem hiding this comment.
What happens if we don't explicitly set CROSSCOMPILING_EMULATOR? Is empty not the default?
There was a problem hiding this comment.
I'm not 100% sure how CMake will treat it given that it's an imported target, but for non-imported executable targets, it wants CROSSCOMPILING_EMULATOR to be defined if the target is used in add_custom_command. So I'm being defensive here.
| project(host-tools) | ||
|
|
||
| # Require standard C++14 | ||
| set(CMAKE_CXX_STANDARD 14) |
There was a problem hiding this comment.
...don't we actually only require C++11?
There was a problem hiding this comment.
IIRC when we went through and looked at our list of supported compilers (GCC 5+, Clang 9+, MSVC 2017+), C++14 was the GCD among them.
|
Build looks clean. Ready to review/land? |
I wanted some input from @shoaibkamil to see if this works with his cross compiling set-up. |
Will give it a try soon and let you know if this works to create a Windows ARM64 Halide.lib when compiling on a Windows x64 machine. |
|
Thanks Shoiab -- note that you should not set |
|
Is this PR still an active concern? |
|
Yes, this is still important. I'm in the middle of rebuilding my windows dev environment so haven't been able to test yet. Will do it ASAP |
Not urgent, just wanted to know the status since this has been dormant for a while |
alexreinking
force-pushed
the
cmake/crosscompile
branch
from
97fcbeb to
ed29832
Compare
This PR updates CMake to build the host tools (currently
binary2cppandbuild_halide_h) externally with a given toolchain (specified by the new CMake variableHalide_HOST_TOOLCHAIN_FILE) when the main project is being cross-compiled.There are some awkward contortions needed to avoid changing the project to an outright superbuild, and I don't see how we can support cross-OS builds (eg. build on Windows, deploy on Linux or vice-versa) without making that change.
We need separate directories for host build tools and end-user/generator build tools. I would vote to move the host tools into
src/toolsand keep/toolsfor user stuff. This makes it easier to organize the CMakeLists.txt.attn: @shoaibkamil -- would appreciate your help testing this since you were the one interested in cross-compiling Halide.
Fixes #5040