Delegate IREE source fetching to shortfin/CMakeLists.txt. #762
Conversation
ScottTodd
force-pushed
the
workflow-iree-ref
branch
from
6003e3d to
d2cb75a
Compare
This will make it easier to update the ref. This will slow workflows down a bit since FetchContent always fetches all submodules.
ScottTodd
force-pushed
the
workflow-iree-ref
branch
from
d2cb75a to
6193390
Compare
| # Keep in sync with SHORTFIN_IREE_GIT_TAG in CMakeLists.txt | ||
| -f https://iree.dev/pip-release-links.html | ||
| iree-base-compiler==3.1.0rc20241204 | ||
| iree-base-runtime==3.1.0rc20241204 |
There was a problem hiding this comment.
@marbre did you experiment with having the CMake file pull from this requirements file?
I guess we can also just add a comment in this other location pointing back here too:
shark-ai/shortfin/CMakeLists.txt
Lines 46 to 47 in 5f08cb2
There was a problem hiding this comment.
A few weeks ago I send #502 and #503, but the former reads in a separate file. However, we should be able to parse the version from a requirements file. I remember we had something very hacky and not really stable in one of the IREE bare-metal Arm workflows but we probably should handle it all inside CMake with a regex.
There was a problem hiding this comment.
We discussed this a bit offline. I think having two locations that point to each other is pretty manageable without any extra tooling support. Having five locations with only some pointing to the others is not. So for now I think keeping the requirements and CMakeLists only loosely linked via comments is sufficient.
| @@ -44,6 +44,7 @@ add_compile_options("$<$<C_COMPILER_ID:MSVC>:/utf-8>") | |||
| add_compile_options("$<$<CXX_COMPILER_ID:MSVC>:/utf-8>") | |||
|
|
|||
| # Pins | |||
| # Keep in sync with requirements-iree-compiler.txt. | |||
There was a problem hiding this comment.
Update this if moving the requirements file in #765.
There was a problem hiding this comment.
- Workflows will be several seconds slower since FetchContent always fetches all submodules
It actually not as bad as we have set
shark-ai/shortfin/CMakeLists.txt
Line 234 in 922b1c2
as well as
shark-ai/shortfin/CMakeLists.txt
Line 243 in 922b1c2
- The
SHORTFIN_IREE_SOURCE_DIRoption is no longer tested
That's the only downside I see but we can read a workflow later if we really need it.
|
Ah, I'd forgotten about that detail of git submodules. Thanks for the reminder! |
This simplification will help with #760. Pros: * Now there are fewer places that use a ref pin * Workflows are now simpler Cons: * ~~Workflows will be several seconds slower since FetchContent always fetches all submodules~~ * The `SHORTFIN_IREE_SOURCE_DIR` option is no longer tested
This simplification will help with #760.
Pros:
Cons:
Workflows will be several seconds slower since FetchContent always fetches all submodulesSHORTFIN_IREE_SOURCE_DIRoption is no longer tested