-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Hexagon] -Build Hexagon runtime components using the Hexagon SDK (Clone of #7671) #7741
[Hexagon] -Build Hexagon runtime components using the Hexagon SDK (Clone of #7671) #7741
Conversation
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.
LGTM pending green
clang-tidy is still failing -- note that changes to the buildbot master have no effect on clang-tidy (which is run via GitHub Actions). EDIT: I think I have a fix. |
So even with the Buildbot fix landed, this still won't work as-is: The rules in FindHexagonSDK.cmake seem to assume that the SDK will always be found... which isn't true for most buildbots (and, in general, for most people building Halide); e.g. the I'm not sure how to fix those rules, unfortunately, but we need to fix them before this can land. |
Thanks @steven-johnson. I was out of office today. Let me look into this on Monday and get back to you here. |
@steven-johnson - This is to take care of the Halide configuration failure in the bots that don't handle hexagon. halide/build_bot#253 |
That buildbot fix is a good one (and I have landed it) but it won't address this issue at all; your fix simply reduces unnecessary build time for LLVM on those bots, but the issue at hand here is that the Halide CMake rules for finding the Hexagon SDK will fail if the SDK isn't found; this is irrelevant to how LLVM is built. |
The revert of the build fix notwithstanding, for the sake of checking my understanding of cmake, isnt it correct that the Hexagon SDK will be searched for only if this condition is true? https://github.com/halide/Halide/pull/7741/files#diff-b54200773c0b5ab32237121a90d2145324228571bce9b10a4c16e1f5361902baR342 A Halide user using Hexagon already needs the SDK in offload mode (use of `.hexagon() |
Yes, but TARGET_HEXAGON only means "we can generate Hexagon code", which shouldn't require the SDK to be present -- it should only require that LLVM is built with Hexagon codegen enabled.
A user only requires it if they are running the correctness tests for Hexagon -- which we don't run on systems that don't have the SDK. |
Ok, I get it. So, this means that we should perhaps have a CMake variable that optionally builds the |
Yes. |
Done. |
buildbot updated, re-running tests here |
Looks like we still have legit build failures: https://buildbot.halide-lang.org/master/#/builders/163/builds/69
|
@steven-johnson - Is it possible to install libffi on buildbot worker? |
If it's just
I don't have strong feelings about this -- it's always preferable to avoid extra requirements, of course, but since this already has one large extra requirement anyway (i.e.: the Hexagon SDK), one more if probably no big deal. (We should add this to a README, of course.) |
Update: looks like both the linuxbots already have |
So it appears that libffi.so.6 is no longer available in Ubuntu 20.04 (which is what our buildbots are running). Various suggestions include manually downloading and installing the Ubuntu 19 version; I'll try that. (Ideally, the Hexagon SDK code would upgrade to use the newer versions at some point :-) |
With libffi.6 installed, new failures: https://buildbot.halide-lang.org/master/#/builders/163/builds/73
|
Thank you very much @steven-johnson for installing libffi.so. Let me dig into the failures now. |
Better still :) The latest version of the hexagon SDK has a 'qaic' binary that doesn't depend on libffi
|
There seem to be problems in the Android NDK on the worker.
OTOH, on my local machine, I do see the toolchain file in the ndk
|
The SDK installed on the buildbots doesn't appear to have the android ndk at all, at least not under
If you can point me at a pristine installer for the HVX SDK, I can do a fresh reinstall if we think that might help. |
Thank you, sounds good. Let me get an installer from our SDK team. |
@steven-johnson - There are two different ways to go about this.
** I don't know why it wasn't there the first time we set this up.
The second option will be better in the long term when we (soon) want to update the SDK. |
I installed the unsupported NDK. Trying again. |
Thank you very much. The test passed (https://buildbot.halide-lang.org/master/#/builders/163/builds/84) and the failure in https://buildbot.halide-lang.org/master/#/builders/154/builds/93 is unrelated. |
…one of halide#7671) (halide#7741) * Add CMakeLists.txt to build the hexagon_remote runtime. * Print an error message if libhalide_hexagon_host.so is not found. * Fix case mismatch in hexagon_remote/CMakeLists.txt * Remove some code that had been commented out in hexagon_remote/CMakeLists.txt * Remove unused argument in macro in hexagon_remote/CMakeLists.txt * add find module for Hexagon * move more variables to find module * Build binary modules with ExternalProject * group platform-speicifc sources into subdirectories * Pass HEXAGON_TOOLS_ROOT, too * Use the desired layout for the build-tree artifacts * Use SYSTEM for Hexagon SDK include dirs * trigger buildbots * Ignore code in src/runtime/hexagon_remote/bin/src for clang-tidy * Just skip hexagon_remote entirely for Halide_CLANG_TIDY_BUILD * Add an option to enable the building of the hexagon remote runtime --------- Co-authored-by: Alex Reinking <[email protected]> Co-authored-by: Steven Johnson <[email protected]>
This is work by @alexreinking. This is essentially #7671 with main merged into it to fix clang-tidy and clang-format issues. I am opening this new PR because I dont have access to his branch.
Here is the opening comment from his PR.
This is a reimplementation of #7659 that uses ExternalProject and the Hexagon SDK-provided toolchain files. This will require relatively less maintenance when porting the build between Hexagon SDK versions.
Notable changes:
The source files in src/runtime/hexagon_remote have been arranged into subdirectories qurt and android based on which toolchain must compile them. This avoids a horrible use of recursive CMake.
To-do:
Future work: