From 4e8b647ac1662decd1dc1ba480524ae1f57dc806 Mon Sep 17 00:00:00 2001 From: Nick Sarnie <7064844+sarnex@users.noreply.github.com> Date: Sat, 14 Dec 2024 04:41:00 +0900 Subject: [PATCH] [CI] Use oneAPI for Windows postcommit builds (#16355) Getting this working was an unbelievable amount of work. I hit so many weird issues caused by dumb Windows things (case insensitivity, cmd vs bash, bugs in cmake/ninja/sccache themselves) but finally I got something that consistently passes. You can see the funniest dumb Windows issue in the comment in the CMake file change. The basic idea is to extend the Windows build workflow to allow using `icx`, add an action to setup the oneAPI environment, call it from the build action and the test action (since we need some of the shared libs on path even for testing), and fix some oneAPI-only build issues. We need to do some Powershell magic after calling the oneAPI setup bat script because the environment variables get lost since it's run in a subprocess. We also switch to `ccache` instead of `sccache`, because `sccache` doesn't support `icx` (actually neither does `ccache`, but I added it upstream [here](https://github.com/ccache/ccache/pull/1533) and we are using a local build of `ccache`, it was easier to add support to `ccache`). Manually tested it to confirmed cache reads and write are working as expected. At some point we should probably investigate why some tests fail or some compiler flags are required with `icx` only and not `cl`, but let's do that as separate work because I am done with Windows for now. I already set up all Windows runners used here to work with this change. Next I'll add oneAPI builds on Linux, which I really hope is easier. --------- Signed-off-by: Sarnie, Nick --- .github/workflows/sycl-post-commit.yml | 7 ++- .github/workflows/sycl-windows-build.yml | 54 ++++++++++++++----- .github/workflows/sycl-windows-run-tests.yml | 23 +++++--- .../setup_windows_oneapi_env/action.yml | 28 ++++++++++ sycl/source/CMakeLists.txt | 12 +++++ sycl/source/detail/windows_ur.cpp | 18 ------- sycl/unittests/assert/assert.cpp | 8 ++- 7 files changed, 112 insertions(+), 38 deletions(-) create mode 100644 devops/actions/setup_windows_oneapi_env/action.yml diff --git a/.github/workflows/sycl-post-commit.yml b/.github/workflows/sycl-post-commit.yml index 6817693e02be3..89e293736a1bd 100644 --- a/.github/workflows/sycl-post-commit.yml +++ b/.github/workflows/sycl-post-commit.yml @@ -103,7 +103,11 @@ jobs: && success() && github.repository == 'intel/llvm' uses: ./.github/workflows/sycl-windows-build.yml - + with: + compiler: icx + build_configure_extra_args: --cmake-opt=-DCMAKE_C_FLAGS="/clang:-Wno-nonportable-include-path /clang:-Wno-cast-function-type-mismatch" --cmake-opt=-DCMAKE_CXX_FLAGS="/clang:-Wno-nonportable-include-path /clang:-Wno-cast-function-type-mismatch" --cmake-opt="-DCMAKE_EXE_LINKER_FLAGS=/manifest:no" --cmake-opt="-DCMAKE_MODULE_LINKER_FLAGS=/manifest:no" --cmake-opt="-DCMAKE_SHARED_LINKER_FLAGS=/manifest:no" + build_cache_suffix: icx + e2e-win: needs: build-win # Continue if build was successful. @@ -117,6 +121,7 @@ jobs: runner: '["Windows","gen12"]' sycl_toolchain_archive: ${{ needs.build-win.outputs.artifact_archive_name }} extra_lit_opts: --param gpu-intel-gen12=True + compiler: icx macos_default: name: macOS diff --git a/.github/workflows/sycl-windows-build.yml b/.github/workflows/sycl-windows-build.yml index 4bd537146bf31..02289522c833c 100644 --- a/.github/workflows/sycl-windows-build.yml +++ b/.github/workflows/sycl-windows-build.yml @@ -10,6 +10,9 @@ on: build_ref: type: string required: false + build_configure_extra_args: + type: string + required: false changes: type: string description: 'Filter matches for the changed files in the PR' @@ -22,6 +25,10 @@ on: description: 'Artifacts retention period' type: string default: 3 + compiler: + type: string + required: false + default: "cl" outputs: build_conclusion: @@ -41,6 +48,9 @@ on: type: choice options: - "default" + build_configure_extra_args: + type: string + required: false artifact_archive_name: type: choice options: @@ -50,6 +60,12 @@ on: type: choice options: - 3 + compiler: + type: choice + options: + - cl + - icx + permissions: read-all jobs: @@ -61,24 +77,30 @@ jobs: outputs: build_conclusion: ${{ steps.build.conclusion }} steps: + - uses: actions/checkout@v4 + with: + path: src + ref: ${{ inputs.build_ref || github.sha }} + fetch-depth: 1 - uses: ilammy/msvc-dev-cmd@0b201ec74fa43914dc39ae48a89fd1d8cb592756 with: arch: amd64 + - name: Setup oneAPI env + uses: ./src/devops/actions/setup_windows_oneapi_env + if: ${{ always() && !cancelled() && inputs.compiler == 'icx' }} - name: Set env run: | git config --system core.longpaths true git config --global core.autocrlf false echo "C:\Program Files\Git\usr\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - echo "SCCACHE_DIR=D:\github\_work\cache\${{ inputs.build_cache_suffix }}" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append - - uses: actions/checkout@v4 - with: - path: src - ref: ${{ inputs.build_ref || github.sha }} - fetch-depth: 1 + echo "CCACHE_DIR=D:\github\_work\cache\${{ inputs.build_cache_suffix }}" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + echo "CCACHE_MAXSIZE=10G" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append - name: Register cleanup after job is finished uses: ./src/devops/actions/cleanup - name: Configure shell: cmd + env: + ARGS: ${{ inputs.build_configure_extra_args }} # TODO switch to clang-cl and lld when this is fixed https://github.com/oneapi-src/level-zero/issues/83 run: | mkdir build @@ -86,12 +108,12 @@ jobs: IF NOT EXIST D:\github\_work\cache MKDIR D:\github\_work\cache IF NOT EXIST D:\github\_work\cache\${{inputs.build_cache_suffix}} MKDIR D:\github\_work\cache\${{inputs.build_cache_suffix}} python.exe src/buildbot/configure.py -o build ^ - --ci-defaults ^ - --cmake-opt="-DCMAKE_C_COMPILER=cl" ^ - --cmake-opt="-DCMAKE_CXX_COMPILER=cl" ^ + --ci-defaults %ARGS% ^ + --cmake-opt="-DCMAKE_C_COMPILER=${{inputs.compiler}}" ^ + --cmake-opt="-DCMAKE_CXX_COMPILER=${{inputs.compiler}}" ^ --cmake-opt="-DCMAKE_INSTALL_PREFIX=%GITHUB_WORKSPACE%\install" ^ - --cmake-opt="-DCMAKE_CXX_COMPILER_LAUNCHER=sccache" ^ - --cmake-opt="-DCMAKE_C_COMPILER_LAUNCHER=sccache" ^ + --cmake-opt="-DCMAKE_CXX_COMPILER_LAUNCHER=ccache" ^ + --cmake-opt="-DCMAKE_C_COMPILER_LAUNCHER=ccache" ^ --cmake-opt="-DLLVM_INSTALL_UTILS=ON" ^ --cmake-opt="-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=SPIRV" - name: Build @@ -101,7 +123,11 @@ jobs: cmake --build build --target sycl-toolchain - name: check-llvm if: always() && !cancelled() && contains(inputs.changes, 'llvm') + shell: bash run: | + if [[ ${{inputs.compiler}} == 'icx' ]]; then + export LIT_FILTER="SYCL" + fi cmake --build build --target check-llvm - name: check-clang if: always() && !cancelled() && contains(inputs.changes, 'clang') @@ -109,8 +135,12 @@ jobs: cmake --build build --target check-clang - name: check-sycl if: always() && !cancelled() && contains(inputs.changes, 'sycl') + shell: bash run: | - cmake --build build --target check-sycl + if [[ ${{inputs.compiler}} == 'icx' ]]; then + export LIT_FILTER_OUT="host_tanpi_double_accuracy" + fi + cmake --build build --target check-sycl - name: check-sycl-unittests if: always() && !cancelled() && contains(inputs.changes, 'sycl') run: | diff --git a/.github/workflows/sycl-windows-run-tests.yml b/.github/workflows/sycl-windows-run-tests.yml index 9186392f8fa46..22104504ae75e 100644 --- a/.github/workflows/sycl-windows-run-tests.yml +++ b/.github/workflows/sycl-windows-run-tests.yml @@ -33,6 +33,11 @@ on: default: '{}' required: False + compiler: + type: string + required: false + default: "cl" + permissions: read-all jobs: @@ -42,20 +47,23 @@ jobs: environment: WindowsCILock env: ${{ fromJSON(inputs.env) }} steps: + # TODO: use cached_checkout + - uses: actions/checkout@v4 + with: + persist-credentials: false + ref: ${{ inputs.ref || github.sha }} + path: llvm - uses: ilammy/msvc-dev-cmd@0b201ec74fa43914dc39ae48a89fd1d8cb592756 with: arch: amd64 + - name: Setup oneAPI env + uses: ./llvm/devops/actions/setup_windows_oneapi_env + if: ${{ always() && !cancelled() && inputs.compiler == 'icx' }} - name: Set env run: | git config --system core.longpaths true git config --global core.autocrlf false echo "C:\Program Files\Git\usr\bin" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - # TODO: use cached_checkout - - uses: actions/checkout@v4 - with: - persist-credentials: false - ref: ${{ inputs.ref || github.sha }} - path: llvm - name: Register cleanup after job is finished uses: ./llvm/devops/actions/cleanup - name: Download compiler toolchain @@ -84,6 +92,9 @@ jobs: shell: bash run: | # Run E2E tests. + if [[ ${{inputs.compiler}} == 'icx' ]]; then + export LIT_FILTER_OUT="compile_on_win_with_mdd" + fi export LIT_OPTS="-v --no-progress-bar --show-unsupported --show-pass --show-xfail --max-time 3600 --time-tests ${{ inputs.extra_lit_opts }}" cmake --build build-e2e --target check-sycl-e2e - name: Detect hung tests diff --git a/devops/actions/setup_windows_oneapi_env/action.yml b/devops/actions/setup_windows_oneapi_env/action.yml new file mode 100644 index 0000000000000..7b7463f697f23 --- /dev/null +++ b/devops/actions/setup_windows_oneapi_env/action.yml @@ -0,0 +1,28 @@ +name: Windows setup oneAPI env + +runs: + using: "composite" + steps: + - name: Setup oneAPI env + shell: powershell + run: | + $batchFilePath = "C:\Program Files (x86)\Intel\oneAPI\setvars.bat" + + $githubEnvFilePath = $env:GITHUB_ENV + + $envBefore = Get-ChildItem Env: | ForEach-Object { "$($_.Name)=$($_.Value)" } + + $envVars = & cmd.exe /c "call `"$batchFilePath`" && set" | Out-String + + $envAfter = $envVars -split "`r`n" | Where-Object { $_ -match "^(.*?)=(.*)$" } + + foreach ($envVar in $envAfter) { + if ($envVar -match "^(.*?)=(.*)$") { + $name = $matches[1] + $value = $matches[2] + $envBeforeVar = $envBefore | Where-Object { $_ -like "$name=*" } + if (-not $envBeforeVar -or $envBeforeVar -ne "$name=$value") { + Add-Content -Path $githubEnvFilePath -Value "$name=$value" + } + } + } diff --git a/sycl/source/CMakeLists.txt b/sycl/source/CMakeLists.txt index 7bb35a9e158cd..d9f8801d45ba5 100644 --- a/sycl/source/CMakeLists.txt +++ b/sycl/source/CMakeLists.txt @@ -222,6 +222,18 @@ function(add_sycl_rt_library LIB_NAME LIB_OBJ_NAME) target_compile_options(${LIB_OBJ_NAME} PRIVATE -Winstantiation-after-specialization) endif() + + # When building using icx on Windows, the VERSION file + # produced by cmake is used in source code + # when including '' because Windows is + # case-insensitive and icx adds the build directory + # to the system header search path. + if (WIN32 AND CMAKE_CXX_COMPILER_ID MATCHES "IntelLLVM") + set(VERSION_FILE "${CMAKE_BINARY_DIR}/VERSION") + if(EXISTS ${VERSION_FILE}) + file(REMOVE ${VERSION_FILE}) + endif() + endif() endfunction(add_sycl_rt_library) set(SYCL_COMMON_SOURCES diff --git a/sycl/source/detail/windows_ur.cpp b/sycl/source/detail/windows_ur.cpp index bac1163d6d3bd..6233d780c1919 100644 --- a/sycl/source/detail/windows_ur.cpp +++ b/sycl/source/detail/windows_ur.cpp @@ -56,24 +56,6 @@ void *getOsLibraryFuncAddress(void *Library, const std::string &FunctionName) { GetProcAddress((HMODULE)Library, FunctionName.c_str())); } -static std::filesystem::path getCurrentDSODirPath() { - wchar_t Path[MAX_PATH]; - auto Handle = - getOSModuleHandle(reinterpret_cast(&getCurrentDSODirPath)); - DWORD Ret = GetModuleFileName( - reinterpret_cast(ExeModuleHandle == Handle ? 0 : Handle), Path, - MAX_PATH); - assert(Ret < MAX_PATH && "Path is longer than MAX_PATH?"); - assert(Ret > 0 && "GetModuleFileName failed"); - (void)Ret; - - BOOL RetCode = PathRemoveFileSpec(Path); - assert(RetCode && "PathRemoveFileSpec failed"); - (void)RetCode; - - return std::filesystem::path(Path); -} - void *getURLoaderLibrary() { return getPreloadedURLib(); } } // namespace ur diff --git a/sycl/unittests/assert/assert.cpp b/sycl/unittests/assert/assert.cpp index e11184d3a24d2..59b0764d60619 100644 --- a/sycl/unittests/assert/assert.cpp +++ b/sycl/unittests/assert/assert.cpp @@ -145,10 +145,12 @@ static AssertHappened ExpectedToOutput = { }; static constexpr int KernelLaunchCounterBase = 0; -static int KernelLaunchCounter = KernelLaunchCounterBase; static constexpr int MemoryMapCounterBase = 1000; static int MemoryMapCounter = MemoryMapCounterBase; +#ifndef _WIN32 +static int KernelLaunchCounter = KernelLaunchCounterBase; static constexpr int PauseWaitOnIdx = KernelLaunchCounterBase + 1; +#endif // Mock redifinitions static ur_result_t redefinedKernelGetGroupInfoAfter(void *pParams) { @@ -167,6 +169,7 @@ static ur_result_t redefinedKernelGetGroupInfoAfter(void *pParams) { return UR_RESULT_SUCCESS; } +#ifndef _WIN32 static ur_result_t redefinedEnqueueKernelLaunchAfter(void *pParams) { auto params = *static_cast(pParams); static ur_event_handle_t UserKernelEvent = **params.pphEvent; @@ -197,6 +200,7 @@ static ur_result_t redefinedEventWaitPositive(void *pParams) { printf("Waiting for events %i, %i\n", EventIdx1, EventIdx2); return UR_RESULT_SUCCESS; } +#endif static ur_result_t redefinedEventWaitNegative(void *pParams) { auto params = *static_cast(pParams); @@ -223,6 +227,7 @@ static ur_result_t redefinedEnqueueMemBufferMapAfter(void *pParams) { return UR_RESULT_SUCCESS; } +#ifndef _WIN32 static void setupMock(sycl::unittest::UrMock<> &Mock) { using namespace sycl::detail; mock::getCallbacks().set_after_callback("urKernelGetGroupInfo", @@ -234,6 +239,7 @@ static void setupMock(sycl::unittest::UrMock<> &Mock) { mock::getCallbacks().set_before_callback("urEventWait", &redefinedEventWaitPositive); } +#endif namespace TestInteropKernel { const sycl::context *Context = nullptr;