Repository navigation
[SYCL] Deprecate -fsycl-targets=spir64_gen in favor of intel_gpu_<arch> - #23441
srividya-sundaram wants to merge 21 commits into
Conversation
E2E tests compiled for Intel GPUs ahead of time with
'-fsycl-targets=spir64_gen -Xsycl-target-backend=spir64_gen "-device *"'.
That spelling is being deprecated in favor of
'-fsycl-targets=intel_gpu_<arch>', which has no "all devices" equivalent
of '-device *', so the tests need to be told which architecture to build
for.
Replace the gpu_aot_target_opts lit parameter (GPU_AOT_TARGET_OPTS in
CMake), which only carried the '-device' value for spir64_gen, with
gpu_aot_target (GPU_AOT_TARGET). It is a -fsycl-targets value exposed to
tests as %{gpu_aot_target}. When unset, it defaults to the Intel GPU
architectures detected via sycl-ls, or to intel_gpu_pvc when none are
detected (e.g. build-only mode). This mirrors how amd_arch is handled
for HIP.
The spir_gen build target now compiles for %{gpu_aot_target}. Tests that
pinned a specific device keep it, spelled as intel_gpu_<arch>.
spir64_gen is deprecated in favor of -fsycl-targets=intel_gpu_<arch>, both as a -fsycl-targets value and as the triple of -Xsycl-target-backend/frontend/linker: - With the old offloading model it is still accepted, with a -Wdeprecated warning. Support is removed together with the old model. - With the new offloading model (--offload-new-driver) it is an error. Resolve intel_gpu_* aliases when selecting the device sanitizer library, so that e.g. intel_gpu_12_60_7 selects the same PVC library as '-device 12.60.7' did. New offloading model driver tests that used spir64_gen are converted to the equivalent intel_gpu_<arch> form; cases that only exercised parsing of '-device' remain covered by the old offloading model tests. Update the user documentation accordingly.
Resolve conflicts with the new %aot_options substitution: - Take the upstream versions of the tests that now use %aot_options. - Have the %aot_options GPU flags use gpu_aot_target instead of spir64_gen and the removed gpu_aot_target_opts, and match on the selected build targets, since spir_gen no longer maps to a spir64_gen triple. - Convert the new spir64_gen RUN lines in sycl-option-ownership.cpp to intel_gpu_pvc.
iclsrc
left a comment
iclsrc
left a comment
There was a problem hiding this comment.
This PR deprecates -fsycl-targets=spir64_gen, and -Xsycl-target-*=spir64_gen, in favor of intel_gpu_<arch>. With the old offloading model it gives a -Wdeprecated warning, and with the new one a hard error. It also switches getSingleBuildTarget() to resolveGenDevice() so that aliases like intel_gpu_12_60_7 and intel_gpu_acm_g10 pick the right ASAN device library, and it moves the clang/e2e tests to intel_gpu_* plus a new %{gpu_aot_target} lit substitution.
The driver change looks right: diagnoseSYCLOptions() runs after setUseNewOffloadingDriver(), -fsycl-targets= and --offload-targets= are both reported, and the error can't be downgraded with -Wno-deprecated.
🔴 Test not updated (file is not in the diff): clang/test/Driver/sycl-windows-device-filename.cpp (REQUIRES: system-windows) still runs -fsycl-targets=spir64_gen --offload-new-driver -Xsycl-target-backend "-device arch1:arch2" / "-device *" without not. It will now fail with the new err_drv_sycl_spir64_gen_new_driver error. A scan of all RUN lines in clang/test/Driver and sycl/test found this file was the only remaining case. The test checks how the driver cleans up made-up -device values in file names, and there's no intel_gpu_<arch> spelling for that. It needs to switch to --no-offload-new-driver (and accept the warning), or be removed or reworked.
See the inline comments for the remaining issues.
- E2E: in build-only mode, default gpu_aot_target to the architectures of the CI run-only runners instead of only intel_gpu_pvc, since binaries built once are run on all of them. - Driver: also diagnose spir64_gen as the triple of -Xspirv-translator=, -Xdevice-post-link= and -Xspirv-to-ir-wrapper=. - Docs: describe how to migrate -device values that have no single intel_gpu_<arch> equivalent. - Convert the remaining new offloading model spir64_gen RUN lines, including the Windows-only sycl-windows-device-filename.cpp.
|
Fixed in 2a6c3a3: switched |
…default The Windows ocloc used in CI no longer supports Gen12 and older GPUs, so building for intel_gpu_tgllp fails there. Windows run-only CI only uses DG2 and BMG runners, so build for those on Windows. Linux still supports Gen12 and keeps intel_gpu_tgllp for its gen12 runner.
SYCL bundle entry IDs use a 3-component triple followed by the target ID, e.g. sycl-spir64_gen-unknown-unknown-pvc. The bundler parsed the target ID as the triple environment and left the target ID empty, so all entries with the same triple matched each other. With several -fsycl-targets=intel_gpu_* (or nvidia_gpu_*) targets and separate compile and link in the old offloading model, every target was then unbundled to the first entry's device code. Take the last component as the target ID when it is not a known environment but is a known offload architecture. A SYCL code object without a target ID stays compatible with any target ID of the same triple, as before.
… other devices
Images built for intel_gpu_<arch> carry a compile_target and are only
selected for that device, unlike spir64_gen -device <arch> images:
- Basic/build_log.cpp needs an image built for another device, so it keeps
spir64_gen with -Wno-deprecated and is not supported with the new
offloading model.
- DeviceCodeSplit/aot-gpu.cpp, NewOffloadDriver/aot-gpu.cpp and
KernelAndProgram/test_cache_jit_aot.cpp don't depend on a specific device,
so build them for %{gpu_aot_target}. Drop the second AOT block of
test_cache_jit_aot.cpp, which only differed by building one image for
two devices.
…nel_aot.cpp spir64_gen with -device * produced a single abstract module. A list of intel_gpu_<arch> targets produces one per architecture.
When an application is compiled for several -fsycl-targets=intel_gpu_* architectures, each device image is present once per architecture, tagged with a compile_target. Device image dependencies and kernel bundle images were only checked against the image triple, so an image built for another architecture could be picked, e.g. a PVC kernel image linked with the TGLLP image of its dependency, failing with "Unable to merge incompatible images". Skip images whose compile_target does not match the device, as is already done when picking the image for a kernel. Images without a compile_target are unaffected. Make the mock device in SYCLBINSerialize a PVC GPU, since its executable bundle is built from an intel_gpu_pvc image.
….cpp
The test runs with %{run-unfiltered-devices}, so on a machine with several
GPUs it can run on a device other than the one %{gpu_aot_target} was
detected for, e.g. the discrete GPU of the ARL runner. Add spir64 so that
such a device falls back to the SPIR-V image, while the AOT image is still
preferred on the device it was built for.
|
Note to reviewers:
E2E test infrastructure
Existing bugs found by the migration Moving tests to several
These two fixes are included here because the migrated tests need them to pass CI. The commits are split along these lines. |
sarnex
left a comment
sarnex
left a comment
There was a problem hiding this comment.
thanks! some initial comments
|
also you probably want to set the note that if you just set the label and restart jobs, the label isn't picked up for whatever reason, you need to merge/rebase/push a new commit before it gets picked up |
srividya-sundaram
added
the
new-offload-model
mdtoguchi
left a comment
mdtoguchi
left a comment
There was a problem hiding this comment.
As stated in your note - we should pull out the bug fixes to separate PRs.
| `nvptx64-nvidia-cuda`, `amdgcn-amd-amdhsa`, `native_cpu`, Intel GPU AOT and | ||
| `spir64_x86_64` targets respectively. `spir_gen` and `spir_x86_64` select the | ||
| AOT-only GPU and CPU targets respectively, rather than the JIT `spir64` | ||
| triple; `spir_gen` compiles for the `GPU_AOT_TARGET`/`gpu_aot_target` Intel |
There was a problem hiding this comment.
Seeing spir_gen being called out has me curious if there are really any users of this, and do we even truly support it?
There was a problem hiding this comment.
if there are really any users of this, and do we even truly support it?
It was added in #23258
AFAIC, no test requires target-spir_gen
This PR only keeps it consistent by making it build for %{gpu_aot_target} (intel_gpu_<arch>) instead of spir64_gen -device ...
Maybe @wenju-he can clarify its current usage/support?
There was a problem hiding this comment.
Yes, target-spir_gen is in active use. It's what the tc opt_use_gpu_aot flow relies on, and #23258 was added to support that flow. It isn't referenced by any test's REQUIRES/XFAIL. The only consumer is the sycl_build_targets=spir_gen param.
I checked this with tc -x efi2_linux64_xmain_sycl -t sycl-in-tree/Basic~bool-test.cpp -l opt_use_gpu_aot -r none:
- With [SYCL][E2E] Add target-spir_gen and target-spir_x86_64 AOT build targets #23258: The build step runs lit with
--param level_zero_target=spir_gen --param sycl_build_targets=spir_gen. The compile line isicpx -fsycl -fsycl-targets=spir64_gen -Xsycl-target-backend=spir64_gen '-device 0x0BDA -revision_id 0x2f' ..... The binary contains spir64_gen, and the test passes on PVC. - Without it (I removed the target-spir_gen and target-spir_x86_64 entries from lit.cfg.py and E2EExpr.py, and disabled the format.py branch): the same tc command fails with compfail. The lit log shows ValueError: Runtime features: ['any-target-is-spir_gen'] evaluated to True in build-only.
So dropping target-spir_gen breaks the opt_use_gpu_aot flow.
Switching it to %{gpu_aot_target} (intel_gpu_) is fine, as long as that flow still produces an AOT image.
Rather than listing the architectures of the CI runners in lit.cfg.py, have the CI build workflows pass them via --param gpu_aot_target, next to the runner lists they need to match. Without a detected Intel GPU or the parameter, lit now defaults to intel_gpu_dg2 and says so.
gpu_aot_target_opts (GPU_AOT_TARGET_OPTS) passes additional backend options to the AOT compilation of each target in gpu_aot_target, e.g. -cl-opt-disable, for the spir_gen build target and %aot_options. The device is now selected by gpu_aot_target, so passing -device is an error.
… -fno-sycl-rdc The -fno-sycl-rdc compile step (--emit-fatbin-only) wrapped the images of each target group separately and then expected a single output, so a compile for several architectures of one triple, e.g. -fsycl-targets=intel_gpu_pvc,intel_gpu_dg2, failed with "Expect single output from the SYCL device linker". Collect the images of all groups and wrap them into a single wrapper module, as is done for the other offload kinds. The per-architecture information, such as the compile_target, is carried by each image. Inputs with different target triples, which the driver already rejects with -fno-sycl-rdc, are diagnosed rather than labelled with one triple.
Move the compile_target check into compatibleWithDevice, so that every user of it only considers AOT images built for the device's architecture: device image dependencies, kernel bundle images, and hasCompatibleImage. hasCompatibleImage is used by the default device selector to prefer devices with an image. An image built only for intel_gpu_pvc counted as compatible with any Intel GPU, so on a machine with a TGLLP and a PVC GPU the TGLLP one could be selected, and launching the kernel then failed. Apply the same check when picking an image from a set, as done for device_global accesses, which only asked the backend and so could pick an image built for another architecture.
Resolve conflicts in DeviceCodeSplit/aot-gpu.cpp and NewOffloadDriver/aot-gpu.cpp by taking the upstream %aot_options, which builds for gpu_aot_target on this branch.
…tests They were expected to fail on PVC with the new offloading model because clang-linker-wrapper crashed for -fsycl-targets=spir64_gen with -Xsycl-target-backend=spir64_gen '-device pvc,dg2' (intel#23265). The tests now build with -fsycl-targets=intel_gpu_pvc,intel_gpu_dg2 and pass there.
The test isn't specific to DG2 but was converted from spir64_gen -device dg2 to intel_gpu_dg2. Unlike the spir64_gen image, an intel_gpu_dg2 image is only used on DG2, so the test failed on other GPUs, e.g. on BMG with "has no image for the selected device".
…ot_supported_new_model The diagnostic text is not specific to spir64_gen.
…* aliases (#23494) Fixes #23490. The device sanitizer library was selected by comparing `-fsycl-targets` with the exact strings `intel_gpu_pvc` and `intel_gpu_dg2*`, so aliases such as `intel_gpu_12_60_7` (PVC) or `intel_gpu_12_55_8` and `intel_gpu_acm_g10` (DG2) got the generic library. The alias is now resolved first. Split out of #23441, which moves users from `-device 12.60.7` to `intel_gpu_<arch>`.
Fixes #23489. With several `intel_gpu_*` targets, each device image exists once per architecture, tagged with a `compile_target`. Only the kernel image was checked against the `compile_target`; device image dependencies, kernel bundle images, the default device selector and device_global images only checked the triple (`spir64_gen` for all Intel GPUs). This caused "Unable to merge incompatible images", and the default selector could pick a GPU without a usable image. The `compile_target` check is now done in `compatibleWithDevice` and when picking an image from a set. Images without a `compile_target` are unaffected. Split out of #23441, whose tests need it.
This branch has not been deployed
5 participants
-fsycl-targets=spir64_gen -Xsycl-target-backend "-device <arch>"is deprecated in favor of-fsycl-targets=intel_gpu_<arch>.