Skip to content

Commit

Permalink
[CI] Use oneAPI for Windows postcommit builds (#16355)
Browse files Browse the repository at this point in the history
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](ccache/ccache#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 <[email protected]>
  • Loading branch information
sarnex authored Dec 13, 2024
1 parent f0b7493 commit 4e8b647
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 38 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/sycl-post-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
54 changes: 42 additions & 12 deletions .github/workflows/sycl-windows-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -22,6 +25,10 @@ on:
description: 'Artifacts retention period'
type: string
default: 3
compiler:
type: string
required: false
default: "cl"

outputs:
build_conclusion:
Expand All @@ -41,6 +48,9 @@ on:
type: choice
options:
- "default"
build_configure_extra_args:
type: string
required: false
artifact_archive_name:
type: choice
options:
Expand All @@ -50,6 +60,12 @@ on:
type: choice
options:
- 3
compiler:
type: choice
options:
- cl
- icx

permissions: read-all

jobs:
Expand All @@ -61,37 +77,43 @@ 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
mkdir install
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
Expand All @@ -101,16 +123,24 @@ 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')
run: |
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: |
Expand Down
23 changes: 17 additions & 6 deletions .github/workflows/sycl-windows-run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ on:
default: '{}'
required: False

compiler:
type: string
required: false
default: "cl"

permissions: read-all

jobs:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions devops/actions/setup_windows_oneapi_env/action.yml
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
12 changes: 12 additions & 0 deletions sycl/source/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<version>' 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
Expand Down
18 changes: 0 additions & 18 deletions sycl/source/detail/windows_ur.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<void *>(&getCurrentDSODirPath));
DWORD Ret = GetModuleFileName(
reinterpret_cast<HMODULE>(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
Expand Down
8 changes: 7 additions & 1 deletion sycl/unittests/assert/assert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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<ur_enqueue_kernel_launch_params_t *>(pParams);
static ur_event_handle_t UserKernelEvent = **params.pphEvent;
Expand Down Expand Up @@ -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<ur_enqueue_events_wait_params_t *>(pParams);
Expand All @@ -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",
Expand All @@ -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;
Expand Down

0 comments on commit 4e8b647

Please sign in to comment.