[Github] Remove call to llvm-project-tests.yml from mlir-spirv-tests.yml

[boomanaiden154]

This will eventually allow for removing llvm-project-tests.yml. This
should significantly reduce the complexity of this workflow (including
the complexity of llvm-project-tests.yml) at the cost of a little bit of
duplication.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Aiden Grossman (boomanaiden154)

Changes

This will eventually allow for removing llvm-project-tests.yml. This
should significantly reduce the complexity of this workflow (including
the complexity of llvm-project-tests.yml) at the cost of a little bit of
duplication.

---

Full diff: https://github.com/llvm/llvm-project/pull/153871.diff

1 Files Affected:

• (modified) .github/workflows/mlir-spirv-tests.yml (+25-6)

diff --git a/.github/workflows/mlir-spirv-tests.yml b/.github/workflows/mlir-spirv-tests.yml
index 48b6c69a61f50..658858feb8814 100644
--- a/.github/workflows/mlir-spirv-tests.yml
+++ b/.github/workflows/mlir-spirv-tests.yml
@@ -24,9 +24,28 @@ jobs:
   check_spirv:
     if: github.repository_owner == 'llvm'
     name: Test MLIR SPIR-V
-    uses: ./.github/workflows/llvm-project-tests.yml
-    with:
-      build_target: check-mlir
-      projects: mlir
-      extra_cmake_args: '-DLLVM_TARGETS_TO_BUILD="host" -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON'
-      os_list: '["ubuntu-24.04"]'
+    runs-on: ubuntu-24.04
+    container:
+      image: ghcr.io/llvm/ci-ubuntu-24.04:latest
+    steps:
+      - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
+      - name: Setup ccache
+        uses: hendrikmuhs/ccache-action@a1209f81afb8c005c13b4296c32e363431bffea5 # v1.2.17
+        with:
+          max-size: 2G
+          key: spirv-mlir-ubuntu-24.04
+          variant: sccache
+      - name: Build and Test
+        run: |
+          mkdir build
+          cmake -GNinja \
+            -S llvm \
+            -B build \
+            -DCMAKE_BUILD_TYPE=Release \
+            -DLLVM_ENABLE_ASSERTIONS=ON \
+            -DCMAKE_C_COMPILER_LAUNCHER=sccache \
+            -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \
+            -DLLVM_TARGETS_TO_BUILD="host" \
+            -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON \
+            -DLLVM_TARGETS_TO_BUILD=mlir
+          ninja -C build check-mlir

[kuhar]

Is the main difference that we use an explicit and dedicated cache now? Also, how did you decide on the 2GB limit? For comparison, my build dir with mlir + spirv tools in release_with_deb_info with asserts is 39GB.

[kuhar]

Do we want to build with clang and lld?

[boomanaiden154]

We already are. This is run in the CI container where we have a peak optimized clang toolchain. CC and CXX are set so that CMake will pick it up by default, and the toolchain is configured to use lld by default.

Although the last run might not have picked up d0b19cf, which is necessary for CMake to use clang by default.

[boomanaiden154]

Is the main difference that we use an explicit and dedicated cache now? Also, how did you decide on the 2GB limit? For comparison, my build dir with mlir + spirv tools in release_with_deb_info with asserts is 39GB.

It is an explicit cache now, but I don't think that's a major difference. The cache hits on workflows that run rarely like this one are pretty low (at least anecdotally, I don't have hard numbers) given we're using way over the storage limit (10GB) for all caches. Github takes a while to evict them though, so sometimes they can stick around.

This is a release build, so the object file size is way smaller. I usually see <1GB for just building LLVM. I don't think MLIR adds that much more.

This is intended to mostly be a NFC change. The main reason to do this is mostly to clean up some technical debt, namely llvm-project-tests.yml. It handles way more cases than what it is being used for in tree, so removing it will reduce quite a bit of complexity in my opinion.

[IgWod-IMG]

I wonder whether we gain enough to justify extra code duplication. Looking at llvm-project-tests.yml it doesn't seem to do that much additional work and it's quite simple. If this improved run time significantly, I'd be up for it, but currently I'm on the fence. I'm a bit worried it's just an extra maintenance burden for us.

Can you elaborate a bit more what are the parts of llvm-project-tests.yml that we gain from removing?

[boomanaiden154]

I wonder whether we gain enough to justify extra code duplication. Looking at llvm-project-tests.yml it doesn't seem to do that much additional work and it's quite simple.

1. There is all the plumbing to actually make the call work.
2. It contains support for multiple platforms when all in-tree callers only use Linux.
3. It contains explicit support for things like libclc which no one uses in-tree.

This will also be the last user of the workflow after #153876 lands.

I don't see how this adds extra maintenance burden on anyone working on MLIR SPIRV. It makes hacking on the workflow simpler because instead of having to dig through Github plumbing, the workflow is now a CMake+ninja invocation that every LLVM developer will understand along with a fully standard cache/checkout action.

If there are any changes common to all the workflows, I'm usually the one making them, and the difficulty of making such changes will stay the same with this patch.

[IgWod-IMG]

Sounds good, thanks for explaining, I wasn't aware this is a part of a larger effort.

I don't see how this adds extra maintenance burden on anyone working on MLIR SPIRV.

I have a small admission to make... I confused you somehow with the person working on SPIR-V MLIR who set up this workflow and was a bit surprised why we want to move away from using something other jobs use and maintain our own script. Now it makes much more sense.