Reland of #108413: [Offload] Introduce offload-tblgen and initial new API implementation #118503
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @jhuber6, sorry for the spam from the various attempts to get this in. I was caught out by the lack of testing and was relying on my single local configuration for testing. I've now tested release and debug builds, the If there's anything else that is likely to go wrong let me know, otherwise I'd appreciate another review and we can hopefully finally land this properly |
jhuber6
approved these changes
Dec 3, 2024
|
If you have a chance, can you check https://lab.llvm.org/staging/#/builders/131/builds/9701 ? Edit: Link to other bot that turned red w/ this patch: https://lab.llvm.org/staging/#/builders/105/builds/10537 |
|
I think both build with |
jplehr
added a commit
that referenced
this pull request
Dec 3, 2024
…tial new API implementation" (#118541) Reverts #118503 Broke bot https://lab.llvm.org/staging/#/builders/131/builds/9701/steps/5/logs/stdio
jplehr
pushed a commit
that referenced
this pull request
Dec 5, 2024
…I implementation (#118614) Reland #118503. Added a fix for builds with `-DBUILD_SHARED_LIBS=ON` (see last commit). Otherwise the changes are identical. --- ### New API Previous discussions at the LLVM/Offload meeting have brought up the need for a new API for exposing the functionality of the plugins. This change introduces a very small subset of a new API, which is primarily for testing the offload tooling and demonstrating how a new API can fit into the existing code base without being too disruptive. Exact designs for these entry points and future additions can be worked out over time. The new API does however introduce the bare minimum functionality to implement device discovery for Unified Runtime and SYCL. This means that the `urinfo` and `sycl-ls` tools can be used on top of Offload. A (rough) implementation of a Unified Runtime adapter (aka plugin) for Offload is available [here](https://github.com/callumfare/unified-runtime/tree/offload_adapter). Our intention is to maintain this and use it to implement and test Offload API changes with SYCL. ### Demoing the new API ```sh # From the runtime build directory $ ninja LibomptUnitTests $ OFFLOAD_TRACE=1 ./offload/unittests/OffloadAPI/offload.unittests ``` ### Open questions and future work * Only some of the available device info is exposed, and not all the possible device queries needed for SYCL are implemented by the plugins. A sensible next step would be to refactor and extend the existing device info queries in the plugins. The existing info queries are all strings, but the new API introduces the ability to return any arbitrary type. * It may be sensible at some point for the plugins to implement the new API directly, and the higher level code on top of it could be made generic, but this is more of a long-term possibility.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Dec 5, 2024
breaks build OMPT related? ld.lld: error: undefined symbol: llvm::omp::target::ompt::enableDeviceTracing(int) b877a533a6d1 (HEAD -> amd-staging) Revert "Reland llvm#118503: [Offload] Introduce offload-tblgen and initial new API implementation (llvm#118614)" Change-Id: I1f4cb8e06ca625c2a539348d897e5436d314fc0a
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Dec 7, 2024
…w API implementation (llvm#118614) Reland llvm#118503. Added a fix for builds with `-DBUILD_SHARED_LIBS=ON` (see last commit). Otherwise the changes are identical. Disables until OMPT adapts to this. Previous discussions at the LLVM/Offload meeting have brought up the need for a new API for exposing the functionality of the plugins. This change introduces a very small subset of a new API, which is primarily for testing the offload tooling and demonstrating how a new API can fit into the existing code base without being too disruptive. Exact designs for these entry points and future additions can be worked out over time. The new API does however introduce the bare minimum functionality to implement device discovery for Unified Runtime and SYCL. This means that the `urinfo` and `sycl-ls` tools can be used on top of Offload. A (rough) implementation of a Unified Runtime adapter (aka plugin) for Offload is available [here](https://github.com/callumfare/unified-runtime/tree/offload_adapter). Our intention is to maintain this and use it to implement and test Offload API changes with SYCL. ```sh $ ninja LibomptUnitTests $ OFFLOAD_TRACE=1 ./offload/unittests/OffloadAPI/offload.unittests ``` * Only some of the available device info is exposed, and not all the possible device queries needed for SYCL are implemented by the plugins. A sensible next step would be to refactor and extend the existing device info queries in the plugins. The existing info queries are all strings, but the new API introduces the ability to return any arbitrary type. * It may be sensible at some point for the plugins to implement the new API directly, and the higher level code on top of it could be made generic, but this is more of a long-term possibility. Change-Id: I355ac2cc4122d8023a88bc7f427174e16dfa76b9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is another attempt to reland the changes from #108413
The previous two attempts introduced regressions and were reverted. This PR has been more thoroughly tested with various configurations so shouldn't cause any problems this time. If anyone is aware of any likely remaining problems then please let me know.
The changes are identical other than the fixes contained in the last 5 commits.
New API
Previous discussions at the LLVM/Offload meeting have brought up the need for a new API for exposing the functionality of the plugins. This change introduces a very small subset of a new API, which is primarily for testing the offload tooling and demonstrating how a new API can fit into the existing code base without being too disruptive. Exact designs for these entry points and future additions can be worked out over time.
The new API does however introduce the bare minimum functionality to implement device discovery for Unified Runtime and SYCL. This means that the
urinfoandsycl-lstools can be used on top of Offload. A (rough) implementation of a Unified Runtime adapter (aka plugin) for Offload is available here. Our intention is to maintain this and use it to implement and test Offload API changes with SYCL.Demoing the new API
# From the runtime build directory $ ninja LibomptUnitTests $ OFFLOAD_TRACE=1 ./offload/unittests/OffloadAPI/offload.unittestsOpen questions and future work