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

Reland #2 - [Offload] Introduce offload-tblgen and initial new API implementation (#108413. #117704) #117894

Merged
merged 24 commits into from
Nov 28, 2024

Conversation

callumfare
Copy link
Contributor

Relands #117704, which relanded changes from #108413 - this was reverted due to build issues. The new offload library did not build with LIBOMPTARGET_OMPT_SUPPORT enabled, which was not picked up by pre-merge testing.

The last commit contains the fix; everything else is otherwise identical to the approved PR.


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. 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.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.

@callumfare
Copy link
Contributor Author

callumfare commented Nov 27, 2024

@jhuber6 Sorry for the noise, this is attempt 2 to reland the PR. The previous attempt failed because it wasn't tested with LIBOMPTARGET_OMPT_SUPPORT enabled.

@sommerlukas sommerlukas merged commit 992b000 into llvm:main Nov 28, 2024
7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 28, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building offload at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/11193

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: tools/offload-tblgen/functions_basic.td' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/offload-tblgen -gen-api -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/../../../new-api/API /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td --check-prefix=CHECK-API
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/offload-tblgen -gen-api -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/../../../new-api/API /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td
# .---command stderr------------
# | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td:7:9: error: could not find include file 'APIDefs.td'
# | include "APIDefs.td"
# |         ^
# | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td:7:9: error: Unexpected token at top level
# | include "APIDefs.td"
# |         ^
# `-----------------------------
# error: command failed with exit status: 1
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td --check-prefix=CHECK-API
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/functions_basic.td --check-prefix=CHECK-API
# `-----------------------------
# error: command failed with exit status: 2

--

********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 28, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building offload at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/9293

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: tools/offload-tblgen/default_returns.td' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/offload-tblgen -gen-api -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/../../../new-api/API /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td --check-prefix=CHECK-API
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/offload-tblgen -gen-api -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/../../../new-api/API /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td
# .---command stderr------------
# | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td:7:9: error: could not find include file 'APIDefs.td'
# | include "APIDefs.td"
# |         ^
# | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td:7:9: error: Unexpected token at top level
# | include "APIDefs.td"
# |         ^
# `-----------------------------
# error: command failed with exit status: 1
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td --check-prefix=CHECK-API
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/tools/offload-tblgen/default_returns.td --check-prefix=CHECK-API
# `-----------------------------
# error: command failed with exit status: 2

--

********************


@jplehr
Copy link
Contributor

jplehr commented Nov 28, 2024

Just to be clear: There currently is no pre-commit testing of offload. So, you won't see any issues in pre-commit testing.

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.

5 participants