-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[openvino] update to 2024.5.0 #42259
Conversation
c2570ce
to
250b799
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
The build error from CI pipeline test:
|
And I test all features locally, there are the following errors:
My install command:
|
c256a4d
to
7db379c
Compare
@LilyWangLL may I ask your help? I'm afraid there's something wrong with arm64_windows pipelines as it can't find python3 executable in a few stages. May I ask you to have a look or refer someone else who can help with that? |
Hello everyone, I have an issue with arm64_windows builds, but I suspect that there's something wrong with the build environment. I will remove draft label from this PR. |
arm64-windows:
|
If it needs python, the portfile needs vcpkg_find_acquire_program(PYTHON3)
vcpkg_cmake_configure(
...
"-DPYTHON_EXECUTABLE=${PYTHON3}" |
321c4f9
to
752b281
Compare
@LilyWangLL can this be merged? |
Please fix this failure, it still exists. |
752b281
to
5e819f7
Compare
@LilyWangLL may I ask you to check the build again? |
All features passed with following triplets:
Usage test passed on |
vcpkg_from_github( | ||
OUT_SOURCE_PATH DEP_SOURCE_PATH | ||
REPO gabime/spdlog | ||
REF v1.15.0 | ||
SHA512 3dd98409f4625ae4d46ef5f59a2fc22a6e151a13dba9d37433363e5d84eab7cca73b379eeb637d8f9b1f0f5a42221c0cc9a2a70414dc2b6af6a162e19fba0647 | ||
) | ||
file(REMOVE_RECURSE "${SOURCE_PATH}/thirdparty/level_zero/level-zero/third_party/spdlog_headers/spdlog") | ||
file(COPY "${DEP_SOURCE_PATH}/include/" | ||
DESTINATION "${SOURCE_PATH}/thirdparty/level_zero/level-zero/third_party/spdlog_headers/") |
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.
Can these be replaced with dependency spdlog
?
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.
I tried to replace it with a regular dependency, but it requires having "SYSTEM_SPDLOG=ON" variable in level_zero cmake and for some reason when I set this variable in the root project it did not reach level_zero.
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.
@LilyWangLL changing dependencies of dependencies is not of current PR
let's separate product update vs work on dependencies
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.
I'd rather not add new vendored dependencies to this port. This should use the vcpkg spdlog
port as a dependency.
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.
level-zero has an option to use system spdlog via SYSTEM_SPDLOG
looks like we need to enable it.
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.
looks like it's even better to use level-zero enabled in vcpkg #42904
we can use this patch openvinotoolkit/openvino#27633 to be able to find system version of level-zero
@LilyWangLL @JavierMatosD can we merge current PR as upgrade of OpenVINO to newer version and in next PR will submit patch to vcpkg's level-zero migration?
ports/openvino/portfile.cmake
Outdated
@@ -64,26 +63,28 @@ if(ENABLE_INTEL_CPU) | |||
SHA512 8d6dd319924135b7b22940d623305bf200b812ae64cde79000709de4fad429fbd43794301ef16e6f10ed7132777b7a73e9f30ecae7c030aea80d57d7c0ce4500 | |||
) | |||
file(COPY "${DEP_SOURCE_PATH}/" DESTINATION "${SOURCE_PATH}/src/plugins/intel_cpu/thirdparty/mlas") | |||
endif() | |||
|
|||
if(VCPKG_TARGET_ARCHITECTURE MATCHES "arm") |
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.
why was it moved outside of ENABLE_INTEL_CPU
section?
ARM Compute is required only for CPU
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.
I assumed I need to fix arm64-windows build. It would fail looking up python3 executable (required for scons to compile arm compute library). I fixed python3 lookup just to find out the code for arm cpu branch doesn't execute because of the cpu feature constraint platform: !(windows & arm)
. Because cpu is the only plugin existing for arm I assumed that it doesn't make sense to build openvino entirely without plugins and moved the arm cpu code out of the intel cpu branch.
Maybe I should have just disabled build for arm64-windows (or "arm & windows" how it is defined in cpu feature constraints) entirely.
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.
to build ARM compute for Windows arm6, you need clang-cl compiler and native compilation.
So, currently no way to build CPU plugin for Windows arm64
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.
Then I will disable openvino for windows on arm at all so that it doesn't start arm64-windows build and doesn't trigger the error
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.
applied patch to avoid fail if python is not available.
vcpkg_from_github( | ||
OUT_SOURCE_PATH DEP_SOURCE_PATH | ||
REPO gabime/spdlog | ||
REF v1.15.0 | ||
SHA512 3dd98409f4625ae4d46ef5f59a2fc22a6e151a13dba9d37433363e5d84eab7cca73b379eeb637d8f9b1f0f5a42221c0cc9a2a70414dc2b6af6a162e19fba0647 | ||
) | ||
file(REMOVE_RECURSE "${SOURCE_PATH}/thirdparty/level_zero/level-zero/third_party/spdlog_headers/spdlog") | ||
file(COPY "${DEP_SOURCE_PATH}/include/" | ||
DESTINATION "${SOURCE_PATH}/thirdparty/level_zero/level-zero/third_party/spdlog_headers/") |
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.
@LilyWangLL changing dependencies of dependencies is not of current PR
let's separate product update vs work on dependencies
5e819f7
to
2f1026c
Compare
2f1026c
to
d2bcbcf
Compare
### Details: - See microsoft/vcpkg#42259 (comment)
### Details: - See microsoft/vcpkg#42259 (comment)
vcpkg_from_github( | ||
OUT_SOURCE_PATH DEP_SOURCE_PATH | ||
REPO gabime/spdlog | ||
REF v1.15.0 | ||
SHA512 3dd98409f4625ae4d46ef5f59a2fc22a6e151a13dba9d37433363e5d84eab7cca73b379eeb637d8f9b1f0f5a42221c0cc9a2a70414dc2b6af6a162e19fba0647 | ||
) | ||
file(REMOVE_RECURSE "${SOURCE_PATH}/thirdparty/level_zero/level-zero/third_party/spdlog_headers/spdlog") | ||
file(COPY "${DEP_SOURCE_PATH}/include/" | ||
DESTINATION "${SOURCE_PATH}/thirdparty/level_zero/level-zero/third_party/spdlog_headers/") |
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.
I'd rather not add new vendored dependencies to this port. This should use the vcpkg spdlog
port as a dependency.
Can be closed in favor #43053 |
./vcpkg x-add-version --all
and committing the result.