Build Hexagon runtime components using the Hexagon SDK #7671

alexreinking · 2023-07-07T13:07:55Z

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:

Use SYSTEM includes for the SDK directories.
Decide what to do about the new dependency.
- Should Hexagon be off by default?
- Should this part of the build be controlled by a new switch?
- Should Halide depend on hexagon_runtime?
Update GHA CI (clang-tidy missing Hexagon SDK)
Add install rules for these artifacts (if need be)
- Should/can these be part of our binary distribution?
Update or remove the Makefile for this part.
Remove precompiled binaries from the repository
Update documentation
- Requires Hexagon SDK 4.3.0.0, specifically.

Future work:

Port to latest Hexagon SDK

…ists.txt

steven-johnson · 2023-07-10T17:23:09Z

This PR seems to be massively broken on the buildbots -- is it ready for review?

steven-johnson · 2023-07-10T17:25:51Z

src/runtime/hexagon_host.cpp

+    if (!host_lib) {
+        // This will now cause a more specific error 'halide_error_code_symbol_not_found' down the line.
+        // So, just print this message and continue on instead of returning a generic error here.
+        error(user_context) << "Hexagon: unable to load libhalide_hexagon_host.so";


(1) Depending on how you have halide_error() configured, this may not continue on at all.
(2) The golden rule of Halide Runtime code is that every call to error() or halide_error() must also return a nonzero error code across the runtime API boundary; otherwise, you get different failure modes in different configurations.

If you always want to continue from here, use print() instead of error().

steven-johnson · 2023-07-10T17:26:48Z

src/runtime/hexagon_remote/qurt/halide_remote.cpp

@@ -15,8 +15,8 @@ extern "C" {
 #include "known_symbols.h"
 #include "log.h"

-const int stack_alignment = 128;
-const int stack_size = 1024 * 1024;
+// const int stack_alignment = 128;


Why are these commented out?

They are unused and were emitting warnings.

Either remove them, or leave them in with a comment about why they are still there. (In this case, probably just remove)

pranavb-ca · 2023-07-10T19:23:05Z

This PR seems to be massively broken on the buildbots -- is it ready for review?

The PR here needs the an environment variable set
HEXAGON_SDK_ROOT=/home/halidenightly/Qualcomm/Hexagon_SDK/4.3.0/
that should make the linux cmake bots pass (or at least go beyond the current point of failure)

steven-johnson · 2023-07-10T20:36:51Z

This PR seems to be massively broken on the buildbots -- is it ready for review?

The PR here needs the an environment variable set HEXAGON_SDK_ROOT=/home/halidenightly/Qualcomm/Hexagon_SDK/4.3.0/ that should make the linux cmake bots pass (or at least go beyond the current point of failure)

Want me to do that, or do you want to?

pranavb-ca · 2023-07-10T21:54:53Z

This PR seems to be massively broken on the buildbots -- is it ready for review?

The PR here needs the an environment variable set HEXAGON_SDK_ROOT=/home/halidenightly/Qualcomm/Hexagon_SDK/4.3.0/ that should make the linux cmake bots pass (or at least go beyond the current point of failure)

Want me to do that, or do you want to?

This is the right place to look right? https://github.com/halide/build_bot/blob/master/master/master.cfg
There are a couple of more changes needed than just setting HEXAGON_SDK_ROOT. Let me take a stab at it tonight.

pranavb-ca · 2023-07-11T04:33:24Z

This PR seems to be massively broken on the buildbots -- is it ready for review?

The PR here needs the an environment variable set HEXAGON_SDK_ROOT=/home/halidenightly/Qualcomm/Hexagon_SDK/4.3.0/ that should make the linux cmake bots pass (or at least go beyond the current point of failure)

Want me to do that, or do you want to?

This PR seems to be massively broken on the buildbots -- is it ready for review?

The PR here needs the an environment variable set HEXAGON_SDK_ROOT=/home/halidenightly/Qualcomm/Hexagon_SDK/4.3.0/ that should make the linux cmake bots pass (or at least go beyond the current point of failure)

Want me to do that, or do you want to?

This is the right place to look right? https://github.com/halide/build_bot/blob/master/master/master.cfg There are a couple of more changes needed than just setting HEXAGON_SDK_ROOT. Let me take a stab at it tonight.

@steven-johnson - PTAL -> halide/build_bot#237

pranavb-ca · 2023-07-11T21:59:39Z

There is a problem here that I hadn't thought of earlier.
our windows & osx and ARM builders build Hexagon (because of the underlying LLVM having built the hexagon backend). As a result hexagon_remote will get built as per this PR (because TARGET_HEXAGON will be on). But not all builders have Hexagon related properties, namely HL_HEXAGON_TOOLS set up thereby meaning we cannot set up HEXAGON_SDK_ROOT the way things stand now.

For context, see halide/build_bot#237 (comment)

pranavb-ca · 2023-07-13T16:03:09Z

src/runtime/CMakeLists.txt

@@ -339,3 +339,6 @@ add_library(Halide::Runtime ALIAS Halide_Runtime)
 target_include_directories(Halide_Runtime INTERFACE $<BUILD_INTERFACE:${Halide_BINARY_DIR}/include>)
 set_target_properties(Halide_Runtime PROPERTIES EXPORT_NAME Runtime)

+if (TARGET_HEXAGON)


Since this condition can be true on builders that do not handle hexagon, maybe add another check to see if HEXAGON_SDK_ROOT is set up in the environment. It is not the most elegant thing to do though.

An alternative is to partition targets into required and optional. Optional targets would be ones that are not built on all builders. Then provide a cmake option to turn off optional targets. Using the terminology in Halide/dependencies/cmake/llvm/CMakeLists.txt loosely, the logic would be

set(required_components <targets_built_on_all_builders_eg_x86>) set(optional_components <targets build on some builders only, eg. hexagon>) list(APPEND known_components required_components) list(APPEND known_components optional_components) foreach (comp IN LISTS known_components) string(TOUPPER "TARGET_${comp}" OPTION) string(TOUPPER "WITH_${comp}" DEFINE) if (comp STREQUAL "RISCV" AND LLVM_PACKAGE_VERSION VERSION_LESS 17.0) # We default the RISCV target to OFF for LLVM versions prior to 17.0; # it's not clear how robust and well-tested Halide's RISCV codegen # is with LLVM16, and a great deal of effort is being put into # improving it in LLVM17... so default to off so that people won't # hurt themselves too badly. cmake_dependent_option(${OPTION} "Include ${comp} target" OFF "${comp} IN_LIST LLVM_TARGETS_TO_BUILD" OFF) else () if (comp IN LISTS optional_components) cmake_dependent_option(${OPTION} "Include ${comp} target" ON "${comp} IN_LIST LLVM_TARGETS_TO_BUILD; BUILD_OPTIONAL_COMPONENTS" OFF) else() cmake_dependent_option(${OPTION} "Include ${comp} target" ON "${comp} IN_LIST LLVM_TARGETS_TO_BUILD" OFF) endif() endif () if (${OPTION} OR Halide_SHARED_LLVM) message(STATUS "Enabling ${comp} backend") list(APPEND Halide_LLVM_DEFS $<BUILD_INTERFACE:${DEFINE}>) list(APPEND active_components ${comp}) else () message(STATUS "Disabling ${comp} backend") endif () endforeach ()

In Halide/CMakeLists.txt

option(BUILD_OPTIONAL_COMPONENTS "Set to OFF to not build optional components even if the underlying LLVM has that target enabled" ON)

@alexreinking @steven-johnson - Any thoughts on this?

A simpler solution was to simply not build the hexagon backend in llvm on builders that don't handle hexagon. I have updated my PR -> halide/build_bot#237

pranavb-ca · 2023-07-27T15:48:08Z

@steven-johnson - I have tested this again and it looks good. We can merge this. Following this, we should merge halide/build_bot#237

steven-johnson · 2023-07-27T15:53:31Z

OK, but we have to be sure that this passes the clang-tidy checks first -- currently it does not

steven-johnson · 2023-08-01T16:41:23Z

I think we're still just waiting on the clang-tidy fixes here

pranavb-ca · 2023-08-01T21:11:04Z

I think we're still just waiting on the clang-tidy fixes here

Yes, we only need to merge main into this branch and those errors should get resolved.

steven-johnson · 2023-08-01T21:12:55Z

Yes, we only need to merge main into this branch and those errors should get resolved.

I don't have permission to change your branch, you need to do it

pranavb-ca · 2023-08-02T03:56:15Z

Yes, we only need to merge main into this branch and those errors should get resolved.

I don't have permission to change your branch, you need to do it

Unfortunately, this isn't my branch either. It is Alex's as he was helping me out. I couldn't get in touch with him in the office today. I have sent him a message to merge main into his branch, hopefully he'll be online tomorrow.

…one of #7671) (#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 <quic_areinkin@quicinc.com> Co-authored-by: Steven Johnson <srj@google.com>

steven-johnson · 2023-08-23T18:07:15Z

Is this PR still useful, or has it been subsumed by the other PR for this that landed recently?

steven-johnson · 2023-08-25T20:34:16Z

Is this PR still useful, or has it been subsumed by the other PR for this that landed recently?

pranavb-ca · 2023-08-28T00:41:05Z

Is this PR still useful, or has it been subsumed by the other PR for this that landed recently?

Yes, this PR should be closed because it is subsumed by #7741 which has now been merged.

…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 <quic_areinkin@quicinc.com> Co-authored-by: Steven Johnson <srj@google.com>

pranavb-ca and others added 11 commits June 23, 2023 13:34

Add CMakeLists.txt to build the hexagon_remote runtime.

63db286

Print an error message if libhalide_hexagon_host.so is not found.

85b0665

Fix case mismatch in hexagon_remote/CMakeLists.txt

9042978

Remove some code that had been commented out in hexagon_remote/CMakeL…

bdda49c

…ists.txt

Remove unused argument in macro in hexagon_remote/CMakeLists.txt

c1d3129

add find module for Hexagon

e23e8d7

move more variables to find module

b841b0e

Build binary modules with ExternalProject

2c2c866

group platform-speicifc sources into subdirectories

aa28bd4

Pass HEXAGON_TOOLS_ROOT, too

703a063

Use the desired layout for the build-tree artifacts

dda4129

alexreinking requested review from steven-johnson, abadams and pranavb-ca July 7, 2023 13:08

Use SYSTEM for Hexagon SDK include dirs

012b413

alexreinking added build Issues related to building Halide and with CI hexagon Related to the hexagon (HVX) target release_notes For changes that may warrant a note in README for official releases. labels Jul 7, 2023

alexreinking mentioned this pull request Jul 7, 2023

[Hexagon] - Build the hexagon_remote runtime using cmake #7659

Closed

steven-johnson requested changes Jul 10, 2023

View reviewed changes

pranavb-ca mentioned this pull request Jul 11, 2023

Make changes for cmake based build of hexagon_remote halide/build_bot#237

Merged

pranavb-ca reviewed Jul 13, 2023

View reviewed changes

pranavb-ca mentioned this pull request Jul 28, 2023

[Hexagon] - Fix problems in sim_host.cpp #7725

Merged

pranavb-ca mentioned this pull request Aug 4, 2023

[Hexagon] -Build Hexagon runtime components using the Hexagon SDK (Clone of #7671) #7741

Merged

7 tasks

pranavb-ca closed this Aug 28, 2023

alexreinking mentioned this pull request Jun 19, 2024

How to build libsim_qurt.a by myself? #8286

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build Hexagon runtime components using the Hexagon SDK #7671

Build Hexagon runtime components using the Hexagon SDK #7671

alexreinking commented Jul 7, 2023 •

edited by pranavb-ca

Loading

steven-johnson commented Jul 10, 2023

steven-johnson Jul 10, 2023

pranavb-ca Jul 10, 2023

steven-johnson Jul 10, 2023

alexreinking Jul 10, 2023

steven-johnson Jul 10, 2023

pranavb-ca commented Jul 10, 2023

steven-johnson commented Jul 10, 2023

pranavb-ca commented Jul 10, 2023

pranavb-ca commented Jul 11, 2023

pranavb-ca commented Jul 11, 2023

pranavb-ca Jul 13, 2023

pranavb-ca Jul 13, 2023 •

edited

Loading

pranavb-ca Jul 14, 2023

pranavb-ca Jul 21, 2023

pranavb-ca commented Jul 27, 2023

steven-johnson commented Jul 27, 2023

steven-johnson commented Aug 1, 2023

pranavb-ca commented Aug 1, 2023

steven-johnson commented Aug 1, 2023

pranavb-ca commented Aug 2, 2023

steven-johnson commented Aug 23, 2023

steven-johnson commented Aug 25, 2023

pranavb-ca commented Aug 28, 2023

Build Hexagon runtime components using the Hexagon SDK #7671

Build Hexagon runtime components using the Hexagon SDK #7671

Conversation

alexreinking commented Jul 7, 2023 • edited by pranavb-ca Loading

steven-johnson commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavb-ca commented Jul 10, 2023

steven-johnson commented Jul 10, 2023

pranavb-ca commented Jul 10, 2023

pranavb-ca commented Jul 11, 2023

pranavb-ca commented Jul 11, 2023

Choose a reason for hiding this comment

pranavb-ca Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavb-ca commented Jul 27, 2023

steven-johnson commented Jul 27, 2023

steven-johnson commented Aug 1, 2023

pranavb-ca commented Aug 1, 2023

steven-johnson commented Aug 1, 2023

pranavb-ca commented Aug 2, 2023

steven-johnson commented Aug 23, 2023

steven-johnson commented Aug 25, 2023

pranavb-ca commented Aug 28, 2023

alexreinking commented Jul 7, 2023 •

edited by pranavb-ca

Loading

pranavb-ca Jul 13, 2023 •

edited

Loading