Skip to content

[SYCL] Allow overriding plugin libraries #4067

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 16, 2021
Merged

[SYCL] Allow overriding plugin libraries #4067

merged 3 commits into from
Jul 16, 2021

Conversation

alexbatashev
Copy link
Contributor

@alexbatashev alexbatashev commented Jul 7, 2021 •
edited
Loading

Motivation for this change is to provide ability to replay PI traces: a fake plugin opens a file with trace recording and replies to PI calls with info, acquired from that file. This can be used for debugging SYCL applications runtime when target hardware is not available. Such design allows the fake plugin rely on the fact that the library is only loaded once, which makes such a plugin stateless.

@alexbatashev alexbatashev requested a review from a team as a code owner July 7, 2021 15:22
@smaslov-intel
Copy link
Contributor

What is the motivation for this change, please?
The SYCL RT knows which plugin implements which backend by the plugin library name. Should we maybe start reporting the supported backend through the piPluginInit?

There was originally an idea that plugins loading would be controlled by a config file, or even load all pi_*.so libraries in a pre-defined location, and use all that pass some some correctness check.

bso-intel
bso-intel previously approved these changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@bso-intel bso-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@alexbatashev
Copy link
Contributor Author

What is the motivation for this change, please?

I'm trying to record and replay PI traces. I've successfully used XPTI to capture both call arguments and some memory objects to a trace file. Now, I created a plugin, that reads the file and responses to PI calls with recorded information. Currently, the implementation relies on the fact, that all the plugins are replaced with a single library, so that when dlopen is called, it would be loaded just once. This way the file is read sequentially and everything is very simple.

@smaslov-intel
Copy link
Contributor

Interesting! How do envision these replays to be used?
Is there a design document for the new plugin?

used XPTI to capture both call arguments and some memory objects

So your plugin doesn't have a "state", right? You just rely on the order of PI calls and return what was returned before?
What happens if the order/number of PI calls changes in SYCL RT?

These are all aside questions, and the PR itself LGTM.

smaslov-intel
smaslov-intel previously approved these changes Jul 8, 2021
View reviewed changes
@alexbatashev
Copy link
Contributor Author

Interesting! How do envision these replays to be used?

Mainly, I want to be able to debug my applications (and maybe runtime itself) on a machine, that doesn't have the required hardware.

Is there a design document for the new plugin?

I'm developing it outside of the tree, not sure if it makes sense to have such a thing in this repo. Some draft docs and implementation are here.

So your plugin doesn't have a "state", right? You just rely on the order of PI calls and return what was returned before?
What happens if the order/number of PI calls changes in SYCL RT?

It doesn't. There're two main cases, when this is a thing: multithreading and changing host code. First case is handled by per-thread trace records, the second case is handled by std::terminate() if flow diverges :)

@smaslov-intel
Copy link
Contributor

Thanks for the info.

I want to be able to debug my applications (and maybe runtime itself) on a machine, that doesn't have the required hardware.

SYCL RT is vastly target-agnostic, so probably no HW specific bugs there.
Debugging an app is a possibility, but if you change the app it will do different PI calls, and thus std::terminate.

@alexbatashev alexbatashev dismissed stale reviews from smaslov-intel and bso-intel via d67c069 July 14, 2021 06:35
@bader
Copy link
Contributor

bader commented Jul 16, 2021

@alexbatashev, please, add your discussion with @smaslov-intel in the comments to the description.
Log message must answers questions like "What is the motivation for this change, please?"

@alexbatashev
Copy link
Contributor Author

@alexbatashev, please, add your discussion with @smaslov-intel in the comments to the description.
Log message must answers questions like "What is the motivation for this change, please?"

done

@bader bader merged commit 29f3a92 into intel:sycl Jul 16, 2021
@alexbatashev alexbatashev deleted the plugin_overrides branch July 28, 2021 06:45
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Sep 29, 2021
Revert "[SYCL] Allow overriding plugin libraries (intel#4067)"
9bb55a9 
This reverts commit 29f3a92.
dm-vodopyanov pushed a commit that referenced this pull request Sep 30, 2021
Revert "[SYCL] Allow overriding plugin libraries (#4067)" (#4659)
c408e03 
This reverts commit 29f3a92.

There're ways of achieving the same without SYCL runtime modifications.
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Oct 3, 2021
@alexbatashev
Merge branch 'sycl' into remove_outdated_templates
00da882 
* sycl: (108 commits)
  [SYCL][XPTI] Revisit resource management strategy (intel#4494)
  [SYCL][ESIMD] Fix misprint: ESIMD_L1_FLUASH_RO_DATA -> ESIMD_L1_FLUSH_RO_DATA (intel#4681)
  [SYCL] Make kernel_bundle interop more conformant (intel#4672)
  [SYCL] Submission with kernel parameter ignores set kernel bundle (intel#4667)
  [SYCL] Add support for std::byte to vec class  (intel#4637)
  [BuildBot] Uplift CPU/FPGAEMU RT version for CI Process (intel#4671)
  [SYCL] Fix an error on host when big image is used on opencl:gpu (intel#4668)
  [SYCL] Exclude exported symbols from kernel bundles (intel#4660)
  Revert "[SYCL] Allow overriding plugin libraries (intel#4067)" (intel#4659)
  [SYCL] Handle exceptions on mutually exclusive handler operations (intel#4639)
  [sycl-post-link] Don't split module if function pointer has a user that's not CallInst (intel#4657)
  [SYCL][HIP] Fix MemBufferFill for nvidia platform (intel#4629)
  [SYCL][Doc] Describe DPC++ CUDA install w/ non-standard toolkit loc (intel#4663)
  [SYCL] Fix device code instrumentation (intel#4615)
  Remove myself as a code owner (intel#4653)
  [SYCL] Fix overwriting insert to sub_group_mask (intel#4656)
  [x86][Matrix] Replace packed_a with row_major in matrix testcases (intel#4641)
  [SYCL][Doc] Add device global extension spec (intel#4233)
  [sycl-post-link] Adds property listing exported functions (intel#4626)
  [Driver][SYCL] Do not consider non-archive files for FPGA binary checks (intel#4644)
  ...
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Oct 3, 2021
@alexbatashev
Merge branch 'sycl' into sycl_compile_perf
a4f4c4d 
* sycl: (107 commits)
  [SYCL][XPTI] Revisit resource management strategy (intel#4494)
  [SYCL][ESIMD] Fix misprint: ESIMD_L1_FLUASH_RO_DATA -> ESIMD_L1_FLUSH_RO_DATA (intel#4681)
  [SYCL] Make kernel_bundle interop more conformant (intel#4672)
  [SYCL] Submission with kernel parameter ignores set kernel bundle (intel#4667)
  [SYCL] Add support for std::byte to vec class  (intel#4637)
  [BuildBot] Uplift CPU/FPGAEMU RT version for CI Process (intel#4671)
  [SYCL] Fix an error on host when big image is used on opencl:gpu (intel#4668)
  [SYCL] Exclude exported symbols from kernel bundles (intel#4660)
  Revert "[SYCL] Allow overriding plugin libraries (intel#4067)" (intel#4659)
  [SYCL] Handle exceptions on mutually exclusive handler operations (intel#4639)
  [sycl-post-link] Don't split module if function pointer has a user that's not CallInst (intel#4657)
  [SYCL][HIP] Fix MemBufferFill for nvidia platform (intel#4629)
  [SYCL][Doc] Describe DPC++ CUDA install w/ non-standard toolkit loc (intel#4663)
  [SYCL] Fix device code instrumentation (intel#4615)
  Remove myself as a code owner (intel#4653)
  [SYCL] Fix overwriting insert to sub_group_mask (intel#4656)
  [x86][Matrix] Replace packed_a with row_major in matrix testcases (intel#4641)
  [SYCL][Doc] Add device global extension spec (intel#4233)
  [sycl-post-link] Adds property listing exported functions (intel#4626)
  [Driver][SYCL] Do not consider non-archive files for FPGA binary checks (intel#4644)
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@smaslov-intel smaslov-intel smaslov-intel approved these changes

@bso-intel bso-intel bso-intel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexbatashev @smaslov-intel @bader @bso-intel