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

WIP: COMP: Include Halide v18.0.0 in build system #8

Merged

Conversation

NicerNewerCar
Copy link
Contributor

@NicerNewerCar NicerNewerCar commented Aug 15, 2024

@allemangD
Closes #2

A few questions/notes regarding the implementation:

  1. Should the logic be moved from the main CMakeList to a GetHalide.cmake (or another name) file?
  2. When linking against Halide zlib is required, should we also build/include this in the build system? Or require the user to have this installed already?
  3. Everything is working up until I am linking against Halide::Generator, which throws errors since it cannot find llvm, what are your thoughts here?
[ 70%] Built target HalideFilters
[ 74%] Linking CXX executable /root/ITK-build/bin/HalideFiltersTestDriver
/usr/bin/ld: cannot find -lLLVMAnalysis: No such file or directory
/usr/bin/ld: cannot find -lLLVMBitReader: No such file or directory
/usr/bin/ld: cannot find -lLLVMBitWriter: No such file or directory
/usr/bin/ld: cannot find -lLLVMPasses: No such file or directory
/usr/bin/ld: cannot find -lLLVMObject: No such file or directory
/usr/bin/ld: cannot find -lLLVMOrcShared: No such file or directory
/usr/bin/ld: cannot find -lLLVMOrcTargetProcess: No such file or directory
/usr/bin/ld: cannot find -lLLVMSupport: No such file or directory
/usr/bin/ld: cannot find -lLLVMTargetParser: No such file or directory

So far I have tested this on Linux and Windows.

@NicerNewerCar NicerNewerCar marked this pull request as draft August 15, 2024 15:14
@NicerNewerCar NicerNewerCar force-pushed the pull-in-halide-via-build-system branch from 710cabc to dfcbc3e Compare August 15, 2024 15:52
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@allemangD
Copy link
Collaborator

The issue was two-fold. The ITK filter should not link to Halide::Generator; there must be a generator executable that links to Halide::Generator, and use add_halide_library to actually produce the linkable library and header.

The LLVM linking issues arise because the precompiled Halide static libary does not bundle LLVM, and by default Halide::Generator uses that static library. The precompiled Halide shared library does, as it is used for JIT compilation. So, explicitly setting find_package(... shared) forces the generator to use the shared library and avoid the explicit LLVM dependency.

To test that things link correctly I use the "my_first_generator" example from the tutorial, but I don't do anything with it apart from importing the header.

@allemangD allemangD marked this pull request as ready for review August 16, 2024 20:44
@allemangD
Copy link
Collaborator

allemangD commented Aug 16, 2024

I'm not quite sure why the CI runs are failing... I think it may just be that I had not closed the changes in my initial review, so they were not actually re-run? I just approved those changes and manually re-ran the actions to see what happens.

It looks like for some reason the FetchContent logic is not triggered, so find_package(Halide REQUIRED) fails.

https://open.cdash.org/builds/9838219/configure

@allemangD
Copy link
Collaborator

CI seems inconsistent; I tested locally with an equivalent dashboard.cmake with no errors, and indeed the latest build-test-cxx ubuntu-22.04 succeeded without issue. I'll go ahead and merge/close this and we can resolve the CI inconsistency separately, in parallel with work to proceed with implementation.

@allemangD
Copy link
Collaborator

Normally I would rebase-and-merge, but I'll squash to clean up the "WIP" commits.

@allemangD allemangD merged commit 61a7b3c into KitwareMedical:main Aug 19, 2024
16 of 31 checks passed
@NicerNewerCar NicerNewerCar deleted the pull-in-halide-via-build-system branch August 19, 2024 12:47
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.

Pull in Halide via build system
2 participants