Skip to content
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

Add platform specific repository for xclbins #7712

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Conversation

stsoe
Copy link
Collaborator

@stsoe stsoe commented Sep 19, 2023

Problem solved by the commit

For some use flows xclbins are not provided by the application but instead dynamically loaded from a repository created at sw stack install time. This PR adds support for repository of xclbins though the xrt::xclbin_repository first class object.

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

Support repository of xclbins, where repository path(s) are one of

  • explicit directory path
  • platform specific directory where xclbins are installed
  • configuration option via xrt.ini

The repository can be iterated for all xclbins is available in the repo or it can be asked to load a specific xclbin.
The built-in repository path is platform specific.

The PR also changes the behavior of xrt::xclbin constructed from std::string. The constructor now checks for specified xclbin either as (1) absolute path, (2) relative path to current directory, (3) relative path under user configured repository, or (4) relative path under built-in repository.

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

  • Standard OpenCL tests on Alveo.
  • Hand picked windows tests on MCDM.
  • Debugging.

See tests/xrt/56_xclbin/main.cpp for sample use.

@manikandan-xilinx
Copy link
Collaborator

retest this please. (We tried different backend for pcie/hw tests. looks like there was an issue so retest should work)

@gbuildx
Copy link
Collaborator

gbuildx commented Sep 20, 2023

Build Passed!

@gbuildx
Copy link
Collaborator

gbuildx commented Sep 20, 2023

Build Passed!

@stsoe stsoe added the do not merge hold off on merging label Oct 3, 2023
maxzhen pushed a commit to maxzhen/XRT that referenced this pull request Oct 6, 2023
Ensure that only one legacy command monitor thread (kds_device) is
created for platforms that do not implement create_hw_queue.

The legacy command monitor handles command completion for the enqueued
command execution, mostly OpenCL or native XRT with callback, latter
rarely used.  The APIs used by the monitor are legacy execbuf and
execwait() APIs that operate at the device level.

Testing of Xilinx#7712 found that when a platform supports hardware
contexts, but not hardware queue, it ends up creating multiple
kds_devices (command monitor threads) one for each unique hwctx
handle.  The command monitor does not support a single thread creating
multiple instances and also it is wasteful to use multiple monitor
threads for same device when execbuf and execwait calls go through the
xrt_core::device object anyway.

This PR thus adds logic to ensure that only one kds_device is created
for each xrt_core::device.

For platforms that support hardware queue, new queue specific APIs are
used to manage command execution and as such require a command monitor
per hardware queue, so no changes are made here.  Only legacy non
hardware queue flow is changed.

Signed-off-by: Soren Soe <[email protected]>

Signed-off-by: Soren Soe <[email protected]>
@stsoe stsoe requested review from AShivangi and removed request for uday610, rozumx and vboggara-xilinx October 19, 2023 21:23
@stsoe stsoe removed 2024.1 do not merge hold off on merging labels Oct 19, 2023
@gbuildx
Copy link
Collaborator

gbuildx commented Oct 19, 2023

Build failed :(

1 similar comment
@gbuildx
Copy link
Collaborator

gbuildx commented Oct 20, 2023

Build failed :(

For some use flows xclbins are not provided by the application but instead dynamically loaded from a repository created at sw stack install time. This PR adds support for repository of xclbins though the xrt::xclbin_repository first class object.

Support repository of xclbins, where repository path(s) are one of

- explicit directory path
- platform specific directory where xclbins are installed
- configuration option via xrt.ini

The repository can be iterated for all xclbins available in the repo or it can open a specific xclbin.
The built-in repository path is platform specific.

The PR also changes the behavior of xrt::xclbin constructed from std::string. The constructor now checks for specified xclbin either as (1) absolute path, (2) relative path to current directory, or (3) relative path under built-in repository.

What has been tested and how, request additional testing if necessary
Standard OpenCL tests on Alveo.
Hand picked windows tests on MCDM.
Debugging.
See tests/xrt/56_xclbin/main.cpp for sample use.

Signed-off-by: Soren Soe <[email protected]>
- xrtdeps.sh installs gcc-toolset-9-toolchain
- build.sh switches to gcc9 when building on CentOS8
@gbuildx
Copy link
Collaborator

gbuildx commented Oct 20, 2023

Build Passed!

@stsoe stsoe merged commit d152377 into Xilinx:master Oct 20, 2023
3 checks passed
@stsoe stsoe deleted the xclbin_repo branch October 20, 2023 14:23
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