Skip to content
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

Improve support for Arm baremetal compilation and runtime #7286

Merged
merged 9 commits into from
Feb 7, 2023

Conversation

stevesuzuki-arm
Copy link
Contributor

@stevesuzuki-arm stevesuzuki-arm commented Jan 17, 2023

  • Add Target feature "semihosting" mode for baremetal runtime
  • Fix error of aligned_alloc() when compiled by Arm GNU toolchain
  • Add an example app HelloBaremetal to guide how to cross-compile for baremetal target with CMake

- Add Target feature "semihosting" mode for baremetal runtime
- Fix error of aligned_alloc() when compiled by Arm GNU toolchain
@stevesuzuki-arm

This comment was marked as outdated.

Copy link
Contributor

@steven-johnson steven-johnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to provide a better definition of "Semihosting" somewhere in the code; if you didn't have access to this PR, it's not at all obvious (IMHO) what it means or where it's supported. (Otherwise LGTM).

@stevesuzuki-arm
Copy link
Contributor Author

stevesuzuki-arm commented Jan 18, 2023

This PR has the dependency on #7282, #7283, #7284, #7285, and #7297

src/runtime/HalideRuntime.h Show resolved Hide resolved
Copy link
Member

@alexreinking alexreinking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a bunch of places here that reinvent the functionality in add_halide_generator which does the cross-compiling "export() and find_package()" already.

@@ -0,0 +1 @@
!enable_neon.s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steven-johnson - we should add some sort of linter / autoformatter to prevent line-ending churn

Comment on lines +20 to +21
# Find Halide. Instead of the package Halide, HalideHelpers is enough as we don't use JIT mode.
find_package(HalideHelpers REQUIRED)
Copy link
Member

@alexreinking alexreinking Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future design note: As you've observed here and as I've described elsewhere, I think the final design I want to implement (not a blocker here) is to make find_package(Halide REQUIRED) do what find_package(HalideHelpers REQUIRED) does today and if users know they want JIT mode they can add that as a component as in find_package(Halide REQUIRED JIT).

Copy link
Member

@alexreinking alexreinking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another round of feedback 🙂

apps/HelloBaremetal/cmake-twice/CMakeLists.txt Outdated Show resolved Hide resolved
src/runtime/HalideBuffer.h Outdated Show resolved Hide resolved
src/runtime/HalideBuffer.h Outdated Show resolved Hide resolved
apps/HelloBaremetal/README.md Outdated Show resolved Hide resolved
apps/HelloBaremetal/cmake-twice/build.sh Outdated Show resolved Hide resolved
apps/HelloBaremetal/cmake-twice/build.sh Outdated Show resolved Hide resolved
apps/HelloBaremetal/cmake-twice/build.sh Outdated Show resolved Hide resolved
apps/HelloBaremetal/cmake-twice/build.sh Outdated Show resolved Hide resolved
@steven-johnson
Copy link
Contributor

Where does this PR stand?

@alexreinking
Copy link
Member

Where does this PR stand?

For this and the others, I need to re-review.

@steven-johnson
Copy link
Contributor

Checking in again, is this just in need of a re-review?

@stevesuzuki-arm
Copy link
Contributor Author

Checking in again, is this just in need of a re-review?

Review is one thing, the other thing is, some updates depending on how #7285 is landed.
It would be small fix if it is landed. The other options is to disable auto-scheduler and leave as TODO for now if we cannot see when #7285 is fixed.

@steven-johnson steven-johnson merged commit ddb515a into halide:main Feb 7, 2023
@stevesuzuki-arm stevesuzuki-arm deleted the improve_arm_baremetal branch February 13, 2023 10:41
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Improve support for Arm baremetal compilation and runtime

- Add Target feature "semihosting" mode for baremetal runtime
- Fix error of aligned_alloc() when compiled by Arm GNU toolchain

* Modify comments for Target feature semihosting

* Add an example app to guide cross-compilation for baremetal target

* Update build steps in HelloBaremetal

* Fix line-ending

* Set CMake variable BAREMETAL in toolchain file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants