Skip to content

[CI] Add backward ABI-compatibility testing to pre-commit #19719

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

Merged
merged 6 commits into from
Aug 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions .github/workflows/sycl-linux-precommit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,27 @@ jobs:

toolchain_artifact: sycl_linux_ubuntu22

compat_read_exclude:
name: Read compatibility testing exclude list
runs-on: [Linux, build]
outputs:
FILTER: ${{ steps.result.outputs.FILTER }}
steps:
- uses: actions/checkout@v4
with:
sparse-checkout: |
devops/
- name: Register cleanup after job is finished
uses: ./devops/actions/cleanup
- id: result
shell: bash
run: |
# Transform to format expected by `llvm-lit --filter-out "pattern1|pattern2|..."`.
# First, remove comments/empty lines, then join lines with "|" as separator.
echo FILTER="$(grep -v '^#\|^\W*$' devops/compat_ci_exclude.sycl-rel-6_2 | paste -sd '|')" >> $GITHUB_OUTPUT

run_prebuilt_e2e_tests:
needs: [build, detect_changes]
needs: [build, detect_changes, compat_read_exclude]
if: ${{ always() && !cancelled() && needs.build.outputs.build_conclusion == 'success' }}
strategy:
fail-fast: false
Expand Down Expand Up @@ -134,13 +153,19 @@ jobs:
target_devices: level_zero:gpu;opencl:gpu;opencl:cpu
extra_lit_opts: --param test-preview-mode=True
e2e_binaries_artifact: e2e_bin_preview
- name: ABI compatibility against sycl-rel-6_2
runner: '["Linux", "pvc"]'
image: ghcr.io/intel/llvm/sycl_prebuilt_tests:sycl-rel-6_2
target_devices: level_zero:gpu
extra_lit_opts: '--param test-preview-mode=False --filter-out "${{ needs.compat_read_exclude.outputs.FILTER }}"'
e2e_binaries_artifact: 'in-container'

uses: ./.github/workflows/sycl-linux-run-tests.yml
with:
name: ${{ matrix.name }}
runner: ${{ matrix.runner }}
image: ${{ matrix.image }}
image_options: ${{ matrix.image_options }}
image_options: ${{ matrix.image_options || '-u 1001 --device=/dev/dri -v /dev/dri/by-path:/dev/dri/by-path --privileged --cap-add SYS_ADMIN' }}
target_devices: ${{ matrix.target_devices }}
extra_lit_opts: ${{ matrix.extra_lit_opts }}
repo_ref: ${{ github.sha }}
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/sycl-linux-run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ on:
description: |
When set in modes other than `run-only` results in artifact upload.
For `run-only` mode, if specified, means downloading pre-built
binaries from the artifact, otherwise they are expected to be part of
the container.
binaries from the artifact. If set to special value `in-container`
then the binaries are expected to be stored in the container image
instead of coming from an artifact.
type: string
default: ''
required: False
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/sycl-windows-precommit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ on:
- '.github/workflows/trivy.yml'
- 'devops/containers/**'
- 'devops/actions/build_container/**'
- 'devops/compat_ci_exclude.sycl-rel-6_2'
- 'unified-runtime/examples/**'
- 'unified-runtime/scripts/**'
- 'unified-runtime/test/**'
Expand Down
8 changes: 4 additions & 4 deletions devops/actions/run-tests/e2e/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ runs:
using: "composite"
steps:
- name: Checkout E2E tests
if: ${{ !(inputs.testing_mode == 'run-only' && inputs.binaries_artifact == '') }}
if: ${{ !(inputs.testing_mode == 'run-only' && inputs.binaries_artifact == 'in-container') }}
uses: actions/checkout@v4
with:
path: llvm
Expand All @@ -33,19 +33,19 @@ runs:
llvm/utils/lit
sycl/test-e2e
- name: Download E2E Binaries
if: ${{ inputs.testing_mode == 'run-only' && inputs.binaries_artifact != '' }}
if: ${{ inputs.testing_mode == 'run-only' && inputs.binaries_artifact != 'in-container' }}
uses: actions/download-artifact@v4
with:
name: ${{ inputs.binaries_artifact }}
- name: Extract E2E Binaries
if: ${{ inputs.testing_mode == 'run-only' && inputs.binaries_artifact != '' }}
if: ${{ inputs.testing_mode == 'run-only' && inputs.binaries_artifact != 'in-container' }}
shell: bash
run: |
mkdir build-e2e
tar -I 'zstd' -xf e2e_binaries.tar.zst -C build-e2e

- name: Extract E2E tests from container image
if: ${{ inputs.testing_mode == 'run-only' && inputs.binaries_artifact == '' }}
if: ${{ inputs.testing_mode == 'run-only' && inputs.binaries_artifact == 'in-container' }}
shell: bash
run: |
mkdir build-e2e llvm
Expand Down
75 changes: 75 additions & 0 deletions devops/compat_ci_exclude.sycl-rel-6_2
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Known OK:

# https://github.com/intel/llvm/pull/18059 made property a no-op, so `FileCheck`
# fails, but that isn't an actual ABI break.
DiscardEvents/discard_events_accessors.cpp
DiscardEvents/discard_events_using_assert_ndebug.cpp
DiscardEvents/discard_events_usm.cpp
DiscardEvents/invalid_event.cpp
DiscardEvents/invalid_event_exceptions.cpp

# https://github.com/intel/llvm/pull/18287
# Throw exception instead of returning garbage
Basic/info.cpp

# Likely OK, but need author to provide justification, get approval/confirmation
# from someone:

# https://github.com/intel/llvm/pull/18565
# This one should have probably be done in multiple PRs, first improving the
# test's CHECKs and then making an actual functional change.
#
# Based on the title, I'd expect to see reduction in number of ur*retain/release
# calls, but it only moves a few ur*release CHECKs, so I'll let the author
# update this explanation.
Comment on lines +18 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alexandr-Konovalov , please upload a PR that updates this comment with explanation how the change wasn't ABI-breaking.

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened [CI] Add clarification for KernelAndProgram/disable-caching change #19822

KernelAndProgram/disable-caching.cpp

# https://github.com/intel/llvm/pull/18253 broke at least some of the pre-built
# E2E binaries, but do we really need to provide backward compatibility for
# binaries built with sanitizers?
Sanitizer

# https://github.com/intel/llvm/pull/19238
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AndreiZibrov , please upload a PR that updates this with an explanation why this was desired (I'd expect something along the lines of allowing ABI breaks in an experimental extension).

Also tagging @AlexeySachkov

Copy link
Contributor

Choose a reason for hiding this comment

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

@aelovikov-intel - Do you have the output of this failure at hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Clarification here: #19774

NonUniformGroups/opportunistic_group.cpp

# https://github.com/intel/llvm/pull/17955, experimental extension
AsyncAlloc/device/async_alloc_from_pool.cpp
AsyncAlloc/device/async_alloc_zero_init.cpp
AsyncAlloc/device/ooo_queue_async_alloc_from_pool.cpp
Comment on lines +35 to +38
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@intel/sycl-graphs-reviewers , can you please check if that break was intentional and if so update the comment here?

Using 2025-06-18 (pass): https://github.com/intel/llvm/actions/runs/16884717829
Using 2025-06-19 (fail): https://github.com/intel/llvm/actions/runs/16884725131


# Need more investigation by the author:

# https://github.com/intel/llvm/pull/18314
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alexandr-Konovalov , looks like your #18314 broke backward compatibility here, see
https://github.com/intel/llvm/actions/runs/16882134385 (using nightly-2025-05-08, pass)
https://github.com/intel/llvm/actions/runs/16882361500 (using nightly-2025-05-09, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, an interesting issue! Opened [SYCL] Add copy/move ctors and assignment operators to sycl::detail::optional #19775.

Assert/assert_in_kernels.cpp
Assert/assert_in_multiple_tus.cpp
Assert/assert_in_multiple_tus_one_ndebug.cpp
Assert/assert_in_one_kernel.cpp
Assert/assert_in_simultaneous_kernels.cpp
Assert/assert_in_simultaneously_multiple_tus.cpp
Assert/assert_in_simultaneously_multiple_tus_one_ndebug.cpp

# https://github.com/intel/llvm/pull/17442
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@steffenlarsen , looks like your #17442 broke backward compatibility here, see
https://github.com/intel/llvm/actions/runs/16882650449 (using nightly-2025-05-01, pass)
https://github.com/intel/llvm/actions/runs/16882716814 (using nightly-2025-05-02, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Copy link
Contributor

@steffenlarsen steffenlarsen Aug 12, 2025

Choose a reason for hiding this comment

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

Will do! Great addition, by the way!

See #19764.

KernelCompiler/opencl.cpp
KernelCompiler/opencl_cache_eviction.cpp
KernelCompiler/opencl_queries.cpp

# https://github.com/intel/llvm/pull/18403 (pulldown, so probably offload-tools
# team should be looking into this)
DeviceImageDependencies/NewOffloadDriver/dynamic.cpp
DeviceImageDependencies/NewOffloadDriver/free_function_kernels.cpp
DeviceImageDependencies/NewOffloadDriver/math_device_lib.cpp
DeviceImageDependencies/NewOffloadDriver/objects.cpp
DeviceImageDependencies/NewOffloadDriver/singleDynamicLibrary.cpp
NewOffloadDriver/aot-gpu.cpp
NewOffloadDriver/buffer.cpp
NewOffloadDriver/multisource.cpp
NewOffloadDriver/spirv_device_obj_smoke.cpp
NewOffloadDriver/split-per-source-main.cpp
NewOffloadDriver/sycl-external-with-optional-features.cpp
Comment on lines +56 to +68
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@intel/dpcpp-tools-reviewers , looks like #18403 pulldown broke backward compatibility here

https://github.com/intel/llvm/actions/runs/16883874414 (using nightly-2025-05-16, pass)
https://github.com/intel/llvm/actions/runs/16883880550 (using nightly-2025-05-17, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was suggested to me to tag @asudarsa here.


# https://github.com/intel/llvm/pull/18277
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@igchor , looks like your #18277 broke backward compatibility here, see

https://github.com/intel/llvm/actions/runs/16883391382 (using nightly-2025-05-19, pass)
https://github.com/intel/llvm/actions/runs/16883503382 (using nightly-2025-05-23, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

InOrderEventsExt/get_last_event.cpp
InorderQueue/in_order_ext_oneapi_submit_barrier.cpp

# https://github.com/intel/llvm/pull/19328
Adapters/interop-level-zero-buffer-ownership.cpp
Comment on lines +74 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@igchor , looks like your #19328 broke backward compatibility here, see

https://github.com/intel/llvm/actions/runs/16883683175 (using nightly-2025-07-10, pass)
https://github.com/intel/llvm/actions/runs/16883690547 (using nightly-2025-07-11, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Loading