Conversation
|
Stack from ghstack (oldest at bottom): |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/12425
Note: Links to docs will display an error until the docs builds have been completed. ❌ 114 New Failures, 1 Cancelled Job, 3 Unrelated FailuresAs of commit 466f669 with merge base b9bb3c1 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
[ghstack-poisoned]
|
Apple build is failing because it can't find libXNNPACK.a: https://github.com/pytorch/executorch/actions/runs/16251337270/job/45881437516?pr=12425#step:9:47386 android build is failing because I didn't propagate cross compilation settings to the ExternalProject: https://github.com/pytorch/executorch/actions/runs/16251337288/job/45881440178?pr=12425#step:15:16927 llama runner linux build is failing because CMake is passing -lXNNPACK instead of the path to the static lib. (this happened during debugging before and it was because the XNNPACK target wasn't defined; probably the same again?) https://github.com/pytorch/executorch/actions/runs/16251337288/job/45881439862?pr=12425 unittest failures don't look related but we'll want to keep an eye on them. |
|
I want to see EXPORT work locally before I come back and fix broader platform support, so I'm going to do that first. Converting to draft in the meantime. |
|
build_frameworks_ios error: |
|
test-llava-runner-linux failure looks like a flake: https://github.com/pytorch/executorch/actions/runs/16301129189/job/46035741014?pr=12425#step:15:15293 . therefore publishing for review |
mcr229
left a comment
There was a problem hiding this comment.
changes in the XNNPACK cmake/dependencies.cmake look good to me
| set(xnnpack_third_party pthreadpool extension_threadpool cpuinfo) | ||
|
|
||
| include(cmake/Dependencies.cmake) | ||
| set(xnnpack_third_party XNNPACK pthreadpool extension_threadpool cpuinfo) |
There was a problem hiding this comment.
curious what does this rearrangment do?
There was a problem hiding this comment.
I just found it weird that we were mutating xnnpack_third_party in cmake/Dependencies.cmake rather than setting it all at once
| set_target_properties( | ||
| XNNPACK PROPERTIES INTERFACE_LINK_LIBRARIES xnnpack-microkernels-prod |
There was a problem hiding this comment.
oh shoot does this mean that any targets that link XNNPACK will also automatically link xnnpack-microkernels-prod as well? So we don't have to explicitly list both XNNPACK and xnnpack-microkernels-prod? I was trying to figure out how to do this for a while, but never figured it out.
There was a problem hiding this comment.
target_link_libraries with visibility PUBLIC or INTERFACE does this for normal libraries, but you need to do it this way for IMPORTED libraries.
| LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
| ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
| PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | ||
| add_library(kleidiai SHARED IMPORTED) |
There was a problem hiding this comment.
i think i merged something that might have conflict here. I just added a check
if (TARGET kleidiai)
before adding the library
There was a problem hiding this comment.
should be fine I think
|
bunch of flakes that need to be rerun to make sure but no actual problems in the 7 failures so far |
|
Apparently FetchContent is preferred over ExternalProject. The thing I wanted from ExternalProject is separating our build from the dependency's build (i.e., not using add_subdirectory), because the XNNPACK dependency ecosystem doesn't support EXPORT. IIUC, we can get that separation using FetchContent by configuring it to support find_package (using either |
I believe this is necessary to support EXPORT. (We probably should not
land this unless we have a PR stacked on top of this that successfully
converts the build to EXPORT; sending this out so I can get CI going)