-
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
Make auto scheduler libs available in HalideHelpers package #7285
Conversation
find_package(HalideHelpers) allows us to use add_halide_library(). But auto scheduler libs are not available unless they are in Halide-Interfaces.cmake. Note: Those libraries are not actually linked to the target application, but need to be available for add_custom_command call.
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.
Not rejecting necessarily, just want to talk about this so we're all on the same page. I've had plans for adding XC support for a while, but they were delayed by my PhD graduation.
@@ -33,7 +33,7 @@ install(TARGETS Halide Halide_Generator Halide_LanguageOptions | |||
|
|||
if (TARGET Halide_Adams2019) | |||
install(TARGETS Halide_Adams2019 Halide_Li2018 Halide_Mullapudi2016 |
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.
This doesn't seem quite right, but might be okay to get people unstuck... it makes me nervous, though.
The package decomposition I want in the end is as follows:
HalideHelpers
(interfaces) includes all the platform-independent CMake helpers, header-only/source-injection libraries, etc.HalideBuildTools
includes all the libraries and executables intended to run on the host system. This is where the (to-be-created)gengen
tool and the autoschedulers should go in the end.Halide
includes the libraries meant to be linked into the target applications.
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.
It's good to know that there is a plan and design which seems to make more sense. I admit this patch is a kind of hack that may not follow the existing design intension. My short term goal is to make it possible to compile the application like #7286 by CMake. If there is any other approach that could be less unacceptable, I'm happy to try and test.
Where does this PR stand? |
What's the status on this? |
Options:
|
Hey @stevesuzuki-arm Really sorry this is so delayed. I'm at the end of my job search/negotiations and it's just taken up way more time than I've budgeted. I'm okay with merging this as a stop-gap, even if it isn't ultimately included in a numbered release (15 will branch before this. 16 will hopefully branch after my more invasive fixes). |
OK, I vote we open a tracking Issue for the proper packaging rework (if one doesn't exist already) and land this. |
#7344 is filed for further rework. |
) * Make auto scheduler libs available in HalideHelpers package find_package(HalideHelpers) allows us to use add_halide_library(). But auto scheduler libs are not available unless they are in Halide-Interfaces.cmake. Note: Those libraries are not actually linked to the target application, but need to be available for add_custom_command call.
find_package(HalideHelpers) allows us to use add_halide_library(). But auto scheduler libs are not available unless they are in Halide-Interfaces.cmake.
Note:
Those libraries are not actually linked to the target application, but need to be available for add_custom_command call.