Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18678
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 1 Cancelled Job, 3 Unrelated FailuresAs of commit f62b9be with merge base 28f3cf3 ( CANCELLED JOB - The following job was cancelled. Please retry:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but was 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. |
suryasidd
requested review from
JacobSzwejbka,
kirklandsign and
larryliu0820
as code owners
meta-cla
bot
added
the
CLA Signed
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
This PR enables Windows support for the OpenVINO backend by adding Windows-specific dynamic library discovery/loading paths and updating build/docs accordingly.
Changes:
- Add Windows OpenVINO runtime auto-discovery in Python bindings (DLL path + DLL directory handling).
- Add Windows dynamic loading support in the C++ OpenVINO runtime (LoadLibrary/GetProcAddress + FreeLibrary).
- Update OpenVINO backend build flags for MSVC and extend README instructions for Windows.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| extension/pybindings/portable_lib.py | Detect openvino_c.dll on Windows and set OPENVINO_LIB_PATH / DLL search directory. |
| backends/openvino/runtime/OpenvinoBackend.cpp | Use LoadLibrary/GetProcAddress on Windows; keep dlopen/dlsym on non-Windows. |
| backends/openvino/runtime/OpenvinoApi.h | Add Windows header include and close library handles via FreeLibrary on Windows. |
| backends/openvino/README.md | Document Windows quick start, env var setup, and build steps. |
| backends/openvino/CMakeLists.txt | Use MSVC-compatible RTTI/exception flags (/GR, /EHsc) on Windows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.error( | ||
| "Failed to add the pybinding extension DLL to the search path. The extension may not work.", | ||
| e, | ||
| ) |
There was a problem hiding this comment.
logger.error() is called with an extra positional argument (the exception) but the message string has no %s placeholder. The logging module will attempt %-formatting and can raise a TypeError, masking the original failure. Use logger.exception(...), or include a %s placeholder / pass exc_info=True.
| target_compile_options(openvino_backend PRIVATE -frtti -fexceptions) | ||
| target_compile_options( | ||
| openvino_backend | ||
| PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/GR /EHsc> |
There was a problem hiding this comment.
The MSVC generator expression contains a space ($<$<CXX_COMPILER_ID:MSVC>:/GR /EHsc>), which CMake will tokenize into separate arguments and can break the generator expression / pass an invalid flag. Split into two generator expressions (one per flag) or otherwise avoid spaces inside the unquoted expression so both /GR and /EHsc are applied correctly on MSVC.
| PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/GR /EHsc> | |
| PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/GR> | |
| $<$<CXX_COMPILER_ID:MSVC>:/EHsc> |
Summary
This PR enables windows support for OpenVINO backend in executorch.