Skip to content

Build fix on windows after https://github.com/Xilinx/XRT/pull/9651 merge#9669

Merged
chvamshi-xilinx merged 2 commits intoXilinx:masterfrom
aktondak:build_fix
Mar 14, 2026
Merged

Build fix on windows after https://github.com/Xilinx/XRT/pull/9651 merge#9669
chvamshi-xilinx merged 2 commits intoXilinx:masterfrom
aktondak:build_fix

Conversation

@aktondak
Copy link
Collaborator

@aktondak aktondak commented Mar 13, 2026

Problem solved by the commit

This PR fixes a build failure we are seeing on windows MCDM build after the merge of #9651 :

"C:\Users\AKTONDAK\Repos\XRT-MCDM-FORK\XRT-MCDM\build\x64\ALL_BUILD.vcxproj" (default target) (1) ->
"C:\Users\AKTONDAK\Repos\XRT-MCDM-FORK\XRT-MCDM\build\x64\src\xrt\src\python\pybind11\pyxrt.vcxproj" (default target) (31) ->
(ClCompile target) ->
  C:\Users\AKTONDAK\Repos\XRT-MCDM-FORK\XRT-MCDM\src\xrt\src\python\pybind11\src\pyxrt.cpp(138,42): error C4996: 'xrt::device::load_xclbin': deprecated, please use hw_context() instead [C:\Users\AKTONDAK\Repos\XRT-MCDM-F 
ORK\XRT-MCDM\build\x64\src\xrt\src\python\pybind11\pyxrt.vcxproj]
  C:\Users\AKTONDAK\Repos\XRT-MCDM-FORK\XRT-MCDM\src\xrt\src\python\pybind11\src\pyxrt.cpp(142,42): error C4996: 'xrt::device::load_xclbin': deprecated, please use hw_context() instead [C:\Users\AKTONDAK\Repos\XRT-MCDM-F 
ORK\XRT-MCDM\build\x64\src\xrt\src\python\pybind11\pyxrt.vcxproj]

The code change suppresses the error/warning and simply calls the deprecated XRT API for the calling python API to handle error logging correctly.

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

None

How problem was solved, alternative solutions (if any) and why they were rejected

  1. As an alternative, we can implement the python binding's load_xclbin API but since pyxrt is designed to be a 1to1 mapping for all C++ APIs, its better to deprecate python API as well.
  2. We could've also explored adding deprecation to the python binding itself but for uniformity, its better for the same C++ handling to trigger when used.

Risks (if any) associated the changes in the commit

Existing python users might start seeing this warning but this is expected.

What has been tested and how, request additional testing if necessary

Not tested as this is only suppressing the build warning and the original deprecation is already merged

Documentation impact (if any)

None

Akshay Tondak added 2 commits March 13, 2026 15:39
Signed-off-by: Akshay Tondak <Akshay.Tondak@amd.com>
Signed-off-by: Akshay Tondak <Akshay.Tondak@amd.com>
@aktondak aktondak requested a review from stsoe March 13, 2026 22:46
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Collaborator

@stsoe stsoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chvamshi-xilinx Let's get this fixed properly

@chvamshi-xilinx chvamshi-xilinx merged commit 8864d62 into Xilinx:master Mar 14, 2026
21 checks passed
@chvamshi-xilinx
Copy link
Collaborator

@chvamshi-xilinx Let's get this fixed properly

Sure @stsoe . @karthdmg-xilinx , Please work on it.

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.

3 participants