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

[L0] move platform cache into the adapter structure #1252

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

pbalcer
Copy link
Contributor

@pbalcer pbalcer commented Jan 16, 2024

The platform cache is a global variable used exclusively by the L0 adapter, and it's protected by a loosely-associated spin lock. However, its destruction is associated with the lifetime of the adapter structure and is deleted the first time adapter refcount reaches 0. This was causing issues whenever the adapter was initialized and destroyed multiple time inside of a single process, which, for example, happens during tests.

This patch fixes the above problem by moving the platform cache from the global state into the adapter structure. This allows for a simpler implementation that no longer requires an explicit lock and instead uses lazy loading (std::call_once).

With this patch, all platform tests are now passing for L0. Closes #824

@pbalcer pbalcer requested review from a team as code owners January 16, 2024 09:32
@pbalcer pbalcer added the level-zero L0 adapter specific issues label Jan 16, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (178e3fc) 15.80% compared to head (6f4c425) 15.80%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1252   +/-   ##
=======================================
  Coverage   15.80%   15.80%           
=======================================
  Files         223      223           
  Lines       31483    31483           
  Branches     3558     3558           
=======================================
  Hits         4975     4975           
  Misses      26457    26457           
  Partials       51       51           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@kbenzie kbenzie left a comment

Choose a reason for hiding this comment

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

LGTM

@pbalcer
Copy link
Contributor Author

pbalcer commented Jan 16, 2024

intel/llvm#12403

Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

LGTM for the level-zero changes, thank you!

steffenlarsen pushed a commit to intel/llvm that referenced this pull request Jan 17, 2024
The ur.hpp and ur.cpp files were moved to the unified runtime repo.

This is currently blocking #12403,
oneapi-src/unified-runtime#1252.
@pbalcer pbalcer added the ready to merge Added to PR's which are ready to merge label Feb 1, 2024
The platform cache is a global variable used exclusively by the
L0 adapter, and it's protected by a loosely-associated spin lock.
However, its destruction is associated with the lifetime of the
adapter structure and is deleted the first time adapter refcount
reaches 0. This was causing issues whenever the adapter was
initialized and destroyed multiple time inside of a single process,
which, for example, happens during tests.

This patch fixes the above problem by moving the platform cache
from the global state into the adapter structure. This allows
for a simpler implementation that no longer requires an explicit
lock and instead uses lazy loading (std::call_once).

With this patch, all platform tests are now passing for L0.
Closes oneapi-src#824
nrspruit pushed a commit to nrspruit/llvm that referenced this pull request Feb 6, 2024
This solves the same issues as intel#12432, but for urDeviceCreateWithNativeHandle.

Includes required oneapi-src/unified-runtime#1252
@nrspruit nrspruit added the v0.8.x Include in the v0.8.x release label Feb 7, 2024
@pbalcer pbalcer merged commit 3943e7e into oneapi-src:main Feb 7, 2024
52 checks passed
aarongreig pushed a commit to aarongreig/unified-runtime that referenced this pull request Feb 9, 2024
[L0] move platform cache into the adapter structure
aarongreig pushed a commit to aarongreig/unified-runtime that referenced this pull request Feb 9, 2024
[L0] move platform cache into the adapter structure
againull pushed a commit to againull/unified-runtime that referenced this pull request Feb 9, 2024
[L0] move platform cache into the adapter structure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.8.x Include in the v0.8.x release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform conformance test hangs on L0 adapter
4 participants