From 4da59508cf9bed0bbbf7044ba6564494d18ee68e Mon Sep 17 00:00:00 2001 From: Luke Drummond Date: Tue, 2 Jul 2024 15:11:57 +0100 Subject: [PATCH 1/2] Get rid of post-commit testing This is an experiment to try and find a good balance of reliable test infrastructure that can't be ignored during merge, tests enough, and runs quickly. It's based on the following principle: Pull request testing should ask and answer the question > If merged, would the proposed change represent a regression to the project? It should answer it to a high confidence, in as short a time as possible It should not: - test things that are unrelated to the user's code - rely on dynamic data or unstable data beyond the author's control (e.g. last night's build results) - be obviously incomplete With the above principles in mind the jobs here are optimized to maximise diversity coverage on available runners, while also minimizing the turnaround time to know whether a merge request is not a regression. There is a conflict here: high coverage relies on lots of hardware, and takes lots of time, but since a long turnaround time discourages contribution we have to make a very fine judgment of what is reasonable. Here we do this by building *once* per native platform toolchain (with potentially differenct configurations to broaden *build* coverage, and afterwards running subsets of tests on each host's available adaptors Mac OS = Apple Clang + libc++; DSO-linkage release build + assertions Win32 = MSVC; release build - no assertions Linux = gcc + libstdc++ release build + assertions This allows us to avoid building with both clang and gcc on a Linux platform - where the coverage gained is marginal (clang has slightly different diagnostics but otherwise doesn't really exercise the input code in a different manner to gcc) - but building with clang on Mac and gcc on Linux does carry a more significant chance of identifying a build regression. Since by default we enable `-Werror` in configuration, this allows us also to minimize the number of warnings merged, but still avoid doing multiple builds per platform. In summary we're not really interested in testing the system compiler, but instead ensuring that a reasonable matrix of configurations (static vs dynamic vs noassert vs assert) do not regress. We hopefully then answer the original question *reasonable confidence in correctness* without the overhead of a brute-force combinations matrix. We make the following assumptions: - the system compiler and standard library are sane. It's tempting to run the end to end tests with multiple host compilers, but divergence in results here most likely indicates a host compiler bug, or bad behaviour in the runtime, which would show up regardless of host codegen. So when the end to end tests fail, they fail due to the user's code, rather than a test of the system compiler. Dogfooding is a noble goal, but it's not suitable for merge request testing, since it introduces an instability for which the author of the PR is *not responsible*. Thus: we hoist dogfooding into the nightly testing which doesn't affect a user's chance of reasonably getting a patch merged before the heat death of the universe. --- .github/workflows/sycl-linux-build.yml | 2 +- .github/workflows/sycl-nightly.yml | 40 ++++++- .github/workflows/sycl-post-commit.yml | 113 ------------------ ...linux-precommit.yml => sycl-precommit.yml} | 93 ++++++++------ 4 files changed, 99 insertions(+), 149 deletions(-) delete mode 100644 .github/workflows/sycl-post-commit.yml rename .github/workflows/{sycl-linux-precommit.yml => sycl-precommit.yml} (70%) diff --git a/.github/workflows/sycl-linux-build.yml b/.github/workflows/sycl-linux-build.yml index 17dcc83aeb437..1afc9727d6264 100644 --- a/.github/workflows/sycl-linux-build.yml +++ b/.github/workflows/sycl-linux-build.yml @@ -14,7 +14,7 @@ on: build_image: type: string required: false - default: "ghcr.io/intel/llvm/ubuntu2204_build:latest" + default: "ghcr.io/intel/llvm/ubuntu2204_build:latest-0300ac924620a51f76c4929794637b82790f12ab" build_ref: type: string required: false diff --git a/.github/workflows/sycl-nightly.yml b/.github/workflows/sycl-nightly.yml index 32a7814fa1c5c..9668b275b9198 100644 --- a/.github/workflows/sycl-nightly.yml +++ b/.github/workflows/sycl-nightly.yml @@ -15,7 +15,14 @@ jobs: with: build_cache_root: "/__w/" build_artifact_suffix: default - build_configure_extra_args: '--hip --cuda' + build_configure_extra_args: + --shared-libs + --no-assertions + --hip + --cuda + --native_cpu + --cmake-opt="-DSYCL_ENABLE_STACK_PRINTING=ON" + --cmake-opt="-DSYCL_LIB_WITH_DEBUG_SYMBOL=ON" merge_ref: '' retention-days: 90 @@ -80,6 +87,37 @@ jobs: image_options: -u 1001 --device=/dev/dri --privileged --cap-add SYS_ADMIN target_devices: opencl:cpu tests_selector: cts + + - name: E2E tests with dev igc on Intel Arc A-Series Graphics + runner: '["Linux", "arc"]' + image: ghcr.io/intel/llvm/ubuntu2204_intel_drivers:devigc + image_options: -u 1001 --device=/dev/dri --privileged --cap-add SYS_ADMIN + target_devices: ext_oneapi_level_zero:gpu;opencl:gpu + reset_gpu: true + install_drivers: >- + ${{ contains(needs.detect_changes.outputs.filters, 'drivers') || + contains(needs.detect_changes.outputs.filters, 'devigccfg') }} + use_dev_igc: ${{ contains(needs.detect_changes.outputs.filters, 'devigccfg') }} + extra_lit_opts: --param matrix-xmx8=True --param gpu-intel-dg2=True + + # Performance tests below. Specifics: + # - only run performance tests (use LIT_FILTER env) + # - ask llvm-lit to show all the output, even for PASS (-a) + # - run in single thread (-j 1) + # - enable the tests in LIT (--param enable-perf-tests=True) + # - run on all available devices. + - name: Perf tests on Intel GEN12 Graphics system + runner: '["Linux", "gen12"]' + env: '{"LIT_FILTER":"PerformanceTests/"}' + extra_lit_opts: -a -j 1 --param enable-perf-tests=True --param gpu-intel-gen12=True + target_devices: all + + - name: Perf tests on Intel Arc A-Series Graphics system + runner: '["Linux", "arc"]' + env: '{"LIT_FILTER":"PerformanceTests/"}' + extra_lit_opts: -a -j 1 --param enable-perf-tests=True + target_devices: all + uses: ./.github/workflows/sycl-linux-run-tests.yml with: name: ${{ matrix.name }} diff --git a/.github/workflows/sycl-post-commit.yml b/.github/workflows/sycl-post-commit.yml deleted file mode 100644 index 518294c591d3f..0000000000000 --- a/.github/workflows/sycl-post-commit.yml +++ /dev/null @@ -1,113 +0,0 @@ -name: SYCL Post Commit - -on: - push: - branches: - - sycl - - sycl-devops-pr/** - - sycl-rel-** - - pull_request: - branches: - - sycl - - sycl-devops-pr/** - paths: - - .github/workflows/sycl-post-commit.yml - - .github/workflows/sycl-linux-build.yml - - .github/workflows/sycl-linux-run-tests.yml - - .github/workflows/sycl-macos-build-and-test.yml - - ./devops/actions/cleanup - - ./devops/actions/cached_checkout - -permissions: read-all - -jobs: - build-lin: - name: Linux (Self build + shared libraries + no-assertions) - if: github.repository == 'intel/llvm' - uses: ./.github/workflows/sycl-linux-build.yml - with: - build_cache_root: "/__w/llvm" - build_cache_suffix: sprod_shared - build_artifact_suffix: sprod_shared - build_configure_extra_args: --shared-libs --no-assertions --hip --cuda --native_cpu --cmake-opt="-DSYCL_ENABLE_STACK_PRINTING=ON" --cmake-opt="-DSYCL_LIB_WITH_DEBUG_SYMBOL=ON" - # Docker image has last nightly pre-installed and added to the PATH - build_image: "ghcr.io/intel/llvm/sycl_ubuntu2204_nightly:build" - cc: clang - cxx: clang++ - merge_ref: '' - - e2e-lin: - needs: [build-lin] - if: ${{ always() && !cancelled() && needs.build-lin.outputs.build_conclusion == 'success' }} - strategy: - fail-fast: false - matrix: - include: - - name: Intel GEN12 Graphics with Level Zero - runner: '["Linux", "gen12"]' - extra_lit_opts: --param gpu-intel-gen12=True - target_devices: ext_oneapi_level_zero:gpu;opencl:fpga - - name: Intel Arc A-Series Graphics with Level Zero - runner: '["Linux", "arc"]' - extra_lit_opts: --param matrix-xmx8=True --param gpu-intel-dg2=True - # Performance tests below. Specifics: - # - only run performance tests (use LIT_FILTER env) - # - ask llvm-lit to show all the output, even for PASS (-a) - # - run in single thread (-j 1) - # - enable the tests in LIT (--param enable-perf-tests=True) - # - run on all available devices. - - name: Perf tests on Intel GEN12 Graphics system - runner: '["Linux", "gen12"]' - env: '{"LIT_FILTER":"PerformanceTests/"}' - extra_lit_opts: -a -j 1 --param enable-perf-tests=True --param gpu-intel-gen12=True - target_devices: all - - name: Perf tests on Intel Arc A-Series Graphics system - runner: '["Linux", "arc"]' - env: '{"LIT_FILTER":"PerformanceTests/"}' - extra_lit_opts: -a -j 1 --param enable-perf-tests=True - target_devices: all - uses: ./.github/workflows/sycl-linux-run-tests.yml - with: - name: ${{ matrix.name }} - runner: ${{ matrix. runner }} - image: ghcr.io/intel/llvm/ubuntu2204_intel_drivers:latest - image_options: -u 1001 --device=/dev/dri --privileged --cap-add SYS_ADMIN - target_devices: ${{ matrix.target_devices || 'ext_oneapi_level_zero:gpu' }} - reset_gpu: true - - extra_lit_opts: ${{ matrix.extra_lit_opts }} - env: ${{ matrix.env || '{}' }} - - ref: ${{ github.sha }} - merge_ref: '' - - sycl_toolchain_artifact: sycl_linux_sprod_shared - sycl_toolchain_archive: ${{ needs.build-lin.outputs.artifact_archive_name }} - sycl_toolchain_decompress_command: ${{ needs.build-lin.outputs.artifact_decompress_command }} - - build-win: - if: | - always() - && success() - && github.repository == 'intel/llvm' - uses: ./.github/workflows/sycl-windows-build.yml - - e2e-win: - needs: build-win - # Continue if build was successful. - if: | - always() - && !cancelled() - && needs.build-win.outputs.build_conclusion == 'success' - uses: ./.github/workflows/sycl-windows-run-tests.yml - with: - name: Intel GEN12 Graphics with Level Zero - runner: '["Windows","gen12"]' - sycl_toolchain_archive: ${{ needs.build-win.outputs.artifact_archive_name }} - extra_lit_opts: --param gpu-intel-gen12=True - - macos_default: - name: macOS - if: github.repository == 'intel/llvm' - uses: ./.github/workflows/sycl-macos-build-and-test.yml diff --git a/.github/workflows/sycl-linux-precommit.yml b/.github/workflows/sycl-precommit.yml similarity index 70% rename from .github/workflows/sycl-linux-precommit.yml rename to .github/workflows/sycl-precommit.yml index 19d106fa23675..15ad74d5412ee 100644 --- a/.github/workflows/sycl-linux-precommit.yml +++ b/.github/workflows/sycl-precommit.yml @@ -1,4 +1,4 @@ -name: SYCL Pre Commit on Linux +name: SYCL Pre Commit on: # We rely on "Fork pull request workflows from outside collaborators" - @@ -18,9 +18,6 @@ on: - 'clang/docs/**' - '**.md' - '**.rst' - - '.github/workflows/sycl-windows-*.yml' - - '.github/workflows/sycl-macos-*.yml' - - '.github/workflows/sycl-nightly.yml' - 'devops/containers/**' - 'devops/actions/build_container/**' @@ -35,7 +32,7 @@ jobs: detect_changes: uses: ./.github/workflows/sycl-detect-changes.yml - build: + build_linux: needs: [detect_changes] if: always() && success() uses: ./.github/workflows/sycl-linux-build.yml @@ -46,12 +43,11 @@ jobs: build_artifact_suffix: "default" build_cache_suffix: "default" changes: ${{ needs.detect_changes.outputs.filters }} - build_image: "ghcr.io/intel/llvm/ubuntu2204_build:latest-0300ac924620a51f76c4929794637b82790f12ab" determine_arc_tests: name: Decide which Arc tests to run - needs: [build, detect_changes] - if: ${{ always() && !cancelled() && needs.build.outputs.build_conclusion == 'success' }} + needs: [build_linux, detect_changes] + if: ${{ always() && !cancelled() && needs.build_linux.outputs.build_conclusion == 'success' }} runs-on: [Linux, aux-tasks] timeout-minutes: 3 outputs: @@ -69,27 +65,38 @@ jobs: else echo 'arc_tests="Matrix/"' >> "$GITHUB_OUTPUT" fi - test: - needs: [build, detect_changes, determine_arc_tests] - if: ${{ always() && !cancelled() && needs.build.outputs.build_conclusion == 'success' }} + + test_linux: + needs: [build_linux, detect_changes, determine_arc_tests] + if: ${{ always() && !cancelled() && needs.build_linux.outputs.build_conclusion == 'success' }} strategy: fail-fast: false matrix: include: - - name: AMD/HIP + # XXX CUDA testing is in a separate workflow because it runs on AWS + # which has different access restrictions/requirements. See + # sycl-linux-precommit.yml if you're looking for the CUDA precommit + # definition + - name: E2E on AMD/HIP runner: '["Linux", "amdgpu"]' image: ghcr.io/intel/llvm/ubuntu2204_build:latest-0300ac924620a51f76c4929794637b82790f12ab image_options: -u 1001 --device=/dev/dri --device=/dev/kfd target_devices: ext_oneapi_hip:gpu - - name: Intel + + - name: E2E on Intel GEN12; level zero GPU, OpenCL CPU runner: '["Linux", "gen12"]' image: ghcr.io/intel/llvm/ubuntu2204_intel_drivers:latest - image_options: -u 1001 --device=/dev/dri --privileged --cap-add SYS_ADMIN target_devices: ext_oneapi_level_zero:gpu;opencl:gpu;opencl:cpu reset_gpu: true install_drivers: ${{ contains(needs.detect_changes.outputs.filters, 'drivers') }} extra_lit_opts: --param gpu-intel-gen12=True - - name: E2E tests on Intel Arc A-Series Graphics + + - name: E2E on Intel GEN12 Graphics; OpenCL GPU, OpenCL FPGA + runner: '["Linux", "gen12"]' + extra_lit_opts: --param gpu-intel-gen12=True + target_devices: opencl:gpu;opencl:fpga + + - name: E2E on Intel Arc A-Series Graphics; Level zero GPU, OpenCL CPU runner: '["Linux", "arc"]' image: ghcr.io/intel/llvm/ubuntu2204_intel_drivers:latest image_options: -u 1001 --device=/dev/dri --privileged --cap-add SYS_ADMIN @@ -98,23 +105,16 @@ jobs: install_drivers: ${{ contains(needs.detect_changes.outputs.filters, 'drivers') }} extra_lit_opts: --param matrix-xmx8=True --param gpu-intel-dg2=True env: '{"LIT_FILTER":${{ needs.determine_arc_tests.outputs.arc_tests }} }' - - name: E2E tests with dev igc on Intel Arc A-Series Graphics + + - name: E2E on Intel Arc A-Series Graphics runner: '["Linux", "arc"]' - image: ghcr.io/intel/llvm/ubuntu2204_intel_drivers:${{ contains(github.event.pull_request.labels.*.name, 'ci-no-devigc') && 'latest' || 'devigc' }} - image_options: -u 1001 --device=/dev/dri --privileged --cap-add SYS_ADMIN - target_devices: ext_oneapi_level_zero:gpu;opencl:gpu - reset_gpu: true - install_drivers: >- - ${{ contains(needs.detect_changes.outputs.filters, 'drivers') || - contains(needs.detect_changes.outputs.filters, 'devigccfg') }} - use_dev_igc: ${{ contains(needs.detect_changes.outputs.filters, 'devigccfg') }} extra_lit_opts: --param matrix-xmx8=True --param gpu-intel-dg2=True - env: '{"LIT_FILTER":${{ needs.determine_arc_tests.outputs.arc_tests }} }' + target_devices: opencl:gpu;opencl:opencl:fpga uses: ./.github/workflows/sycl-linux-run-tests.yml with: name: ${{ matrix.name }} - runner: ${{ matrix. runner }} + runner: ${{ matrix.runner }} image: ${{ matrix.image }} image_options: ${{ matrix.image_options }} target_devices: ${{ matrix.target_devices }} @@ -128,15 +128,40 @@ jobs: merge_ref: '' sycl_toolchain_artifact: sycl_linux_default - sycl_toolchain_archive: ${{ needs.build.outputs.artifact_archive_name }} - sycl_toolchain_decompress_command: ${{ needs.build.outputs.artifact_decompress_command }} + sycl_toolchain_archive: ${{ needs.build_linux.outputs.artifact_archive_name }} + sycl_toolchain_decompress_command: ${{ needs.build_linux.outputs.artifact_decompress_command }} + + build_win: + if: | + always() + && success() + && github.repository == 'intel/llvm' + uses: ./.github/workflows/sycl-windows-build.yml + + test_e2e_win: + needs: build_win + # Continue if build was successful. + if: | + always() + && !cancelled() + && needs.build_win.outputs.build_conclusion == 'success' + uses: ./.github/workflows/sycl-windows-run-tests.yml + with: + name: Intel GEN12 Graphics with Level Zero + runner: '["Windows","gen12"]' + sycl_toolchain_archive: ${{ needs.build_win.outputs.artifact_archive_name }} + extra_lit_opts: --param gpu-intel-gen12=True + build_macos: + name: macOS Build and Test + if: github.repository == 'intel/llvm' + uses: ./.github/workflows/sycl-macos-build-and-test.yml - test-perf: - needs: [build, detect_changes] + test_perf: + needs: [build_linux, detect_changes] if: | always() && !cancelled() - && needs.build.outputs.build_conclusion == 'success' + && needs.build_linux.outputs.build_conclusion == 'success' && (contains(github.event.pull_request.labels.*.name, 'run-perf-tests') || contains(needs.detect_changes.outputs.filters, 'perf-tests')) strategy: @@ -165,7 +190,7 @@ jobs: uses: ./.github/workflows/sycl-linux-run-tests.yml with: name: Perf tests on ${{ matrix.name }} - runner: ${{ matrix. runner }} + runner: ${{ matrix.runner }} image: ${{ matrix.image }} image_options: -u 1001 --privileged --cap-add SYS_ADMIN ${{ matrix.image_extra_opts }} target_devices: all @@ -179,5 +204,5 @@ jobs: merge_ref: '' sycl_toolchain_artifact: sycl_linux_default - sycl_toolchain_archive: ${{ needs.build.outputs.artifact_archive_name }} - sycl_toolchain_decompress_command: ${{ needs.build.outputs.artifact_decompress_command }} + sycl_toolchain_archive: ${{ needs.build_linux.outputs.artifact_archive_name }} + sycl_toolchain_decompress_command: ${{ needs.build_linux.outputs.artifact_decompress_command }} From ce31515bcc44e27559795a49b39007ca9b8033e0 Mon Sep 17 00:00:00 2001 From: Luke Drummond Date: Wed, 3 Jul 2024 12:12:49 +0100 Subject: [PATCH 2/2] fixup! Get rid of post-commit testing --- .github/workflows/sycl-windows-precommit.yml | 58 -------------------- 1 file changed, 58 deletions(-) delete mode 100644 .github/workflows/sycl-windows-precommit.yml diff --git a/.github/workflows/sycl-windows-precommit.yml b/.github/workflows/sycl-windows-precommit.yml deleted file mode 100644 index a82c2b7814d75..0000000000000 --- a/.github/workflows/sycl-windows-precommit.yml +++ /dev/null @@ -1,58 +0,0 @@ -name: SYCL Pre Commit on Windows - -on: - pull_request: - branches: - - sycl - - sycl-devops-pr/** - - llvmspirv_pulldown - - sycl-rel-** - # Do not run builds if changes are only in the following locations - paths-ignore: - - '.github/ISSUE_TEMPLATE/**' - - '.github/CODEOWNERS' - - 'sycl/doc/**' - - 'sycl/gdb/**' - - 'clang/docs/**' - - '**.md' - - '**.rst' - - '.github/workflows/sycl-linux-*.yml' - - '.github/workflows/sycl-precommit-aws.yml' - - '.github/workflows/sycl-macos-*.yml' - - '.github/workflows/sycl-nightly.yml' - - 'devops/containers/**' - - 'devops/actions/build_container/**' - -permissions: read-all - -concurrency: - # Cancel a currently running workflow from the same PR, branch or tag. - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -jobs: - detect_changes: - uses: ./.github/workflows/sycl-detect-changes.yml - - build: - needs: [detect_changes] - if: | - always() && success() - && github.repository == 'intel/llvm' - uses: ./.github/workflows/sycl-windows-build.yml - with: - changes: ${{ needs.detect_changes.outputs.filters }} - - e2e: - needs: build - # Continue if build was successful. - if: | - always() - && !cancelled() - && needs.build.outputs.build_conclusion == 'success' - uses: ./.github/workflows/sycl-windows-run-tests.yml - with: - name: Intel GEN12 Graphics with Level Zero - runner: '["Windows","gen12"]' - sycl_toolchain_archive: ${{ needs.build.outputs.artifact_archive_name }} - extra_lit_opts: --param gpu-intel-gen12=True