Skip to content

Conversation

callumfare
Copy link
Contributor

@callumfare callumfare commented Jul 13, 2023

Corresponding changes to the adapters, pi2ur, etc are in intel/llvm#10349

Summary of new/changed entry points:

  • urInit Adapters no longer perform global state setup in urInit. The loader may still perform setup (layer config etc), so the entry point is still needed, but it might be possible to combine it with the below.
  • urAdapterGet Individual adapters return exactly 1 adapter handle, the loader returns however many adapters are known. Each returned adapter increments the reference count of the adapter by 1. When the reference count goes from 0 to 1, adapters may perform setup of global state (which previously happened in urInit). The underlying object pointed to by the ur_adapter_handle_t should be a single static object in the adapter.
  • urPlatformGet Now explicitly takes the adapters to use as arguments. Multiple adapters can be used in the same call. This allows all platforms available to be queried as before, or more fine selection of platforms from certain adapters.
  • urAdapterRetain/Release Increment or decrement the adapter reference count. When the reference count drops to 0, adapters may perform teardown of global state (which previously happened in urTearDown).
  • urTearDown Adapters no longer perform global state teardown in urTearDown. This entry point may be kept if useful for the loader, otherwise it could be removed.
  • urAdapterGetLastError Previously urPlatformGetLastError. No changes other than it takes an adapter handle rather than platform handle, removing the need to always have a platform handle when a UR entry point is called to be able to process an adapter-specific error.
  • urAdapterGetInfo Can be used to query the backend of the adapter (which is just a duplicate of UR_PLATFORM_BACKEND).

Addresses #714 by allowing better sharing of UR adapters when multiple modules in an application share the UR libraries. They may independently call urAdapterGet safely, or may explicitly pass adapter handles around (retaining/releasing as necessary).

Provides some more programmatic control over which adapters are used by allowing users to query which backend is supported by each adapter, and only fetching platforms from certain adapters.

Fixes intel/llvm#10066 by allowing piPluginGetLastError to be implementable in pi2ur.

@callumfare
Copy link
Contributor Author

I've updated the tests but there are still CI failures. I'll look into those, but the actual spec and loader changes should be ok so feedback is still welcome

desc: "[in][range(0, NumAdapters)] array of adapters to query for platforms."
- type: "uint32_t"
name: "NumAdapters"
desc: "[in] number of adapters pointed to by phAdapters"
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels odd to be passing in all the adapters here as a list, its a different approach to urDeviceGet only taking a single platform. I can see why this is a less intrusive way to do it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I did it this way to keep the changes as unobtrusive as possible, especially since urPlatformGet is in pretty much every example and test.

It also means fewer changes in the loader, which already had a special case for urPlatformGet where it looped over every adapter in the loader context. This PR just changes that to the adapters passed in.

If consistency with the other entry points is important I could look into changing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think its a blocker for this work but perhaps something to look into at a later point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a note to revisit this in #728

@callumfare callumfare force-pushed the callum/adapter_handle branch 2 times, most recently from 65bcfac to a9c2bef Compare July 13, 2023 16:25
Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

In a separate PR we might also want to update urInit and urTearDown documentation.

Copy link
Contributor

@veselypeta veselypeta left a comment

Choose a reason for hiding this comment

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

LGTM

@callumfare callumfare force-pushed the callum/adapter_handle branch 3 times, most recently from 201a30d to 80a1340 Compare July 18, 2023 15:40
@callumfare callumfare marked this pull request as ready for review July 18, 2023 15:42
@callumfare callumfare force-pushed the callum/adapter_handle branch from 80a1340 to 903b6af Compare July 19, 2023 09:47
@callumfare callumfare force-pushed the callum/adapter_handle branch from 903b6af to b279985 Compare July 19, 2023 10:00
@callumfare callumfare merged commit 974a7d6 into oneapi-src:main Jul 19, 2023
aelovikov-intel pushed a commit to intel/llvm that referenced this pull request Jul 28, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src/unified-runtime#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
veselypeta pushed a commit to veselypeta/llvm that referenced this pull request Sep 21, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src/unified-runtime#715 for details)

This fixes intel#10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
fabiomestre pushed a commit to fabiomestre/llvm that referenced this pull request Sep 26, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src/unified-runtime#715 for details)

This fixes intel#10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
fabiomestre pushed a commit to fabiomestre/unified-runtime that referenced this pull request Sep 26, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
fabiomestre pushed a commit that referenced this pull request Sep 27, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
veselypeta pushed a commit to veselypeta/llvm that referenced this pull request Sep 28, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src/unified-runtime#715 for details)

This fixes intel#10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
szadam pushed a commit to szadam/unified-runtime that referenced this pull request Oct 13, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src/unified-runtime#715 for details)

This fixes intel#10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
omarahmed1111 pushed a commit to omarahmed1111/unified-runtime that referenced this pull request Oct 23, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
omarahmed1111 pushed a commit to omarahmed1111/unified-runtime that referenced this pull request Oct 23, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
omarahmed1111 pushed a commit to omarahmed1111/unified-runtime that referenced this pull request Oct 23, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
martygrant pushed a commit to martygrant/unified-runtime that referenced this pull request Nov 9, 2023
Bump the Unified Runtime commit, and make adapter changes needed for the
newly added adapter handles (see
oneapi-src#715 for details)

This fixes #10066 by providing an implementation of
`piPluginGetLastError` in pi2ur.
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.

[SYCL][UR] Fix piPluginGetLastError so native errors are not lost.
4 participants