[SYCL][NewOffloadModel] Refactor SYCL pipeline in clang-linker-wrapper - #22153
Conversation
Extract SYCL offloading steps into named functions within the sycl namespace: runSYCLOffloadingPipeline, postLinkProcessModule, compileAndBundle and compileSYCLDevice. SYCL modifications of linkDevice function has been extracted to the dedicated function - compileSYCLDevice. Revert linkDevice function to its upstream state. Side effect of this revert: hipspv tests get resolved. Related tracker: CMPLRLLVM-73247 Functions containerizeRawImage and writeOffloadFile are moved so that their location corresponds to the upstream version.
|
Note: this refactoring is done in scope of the adding the support for |
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
There was a problem hiding this comment.
quite a lot of CI failed. Is it caused by this patch?
thank you for aligning the code to upstream! |
There was a problem hiding this comment.
Pull request overview
This PR refactors the SYCL offloading flow in clang-linker-wrapper by extracting the SYCL-specific pipeline into named helper functions under the sycl namespace, while restoring the non-SYCL linkDevice path closer to upstream behavior. It also re-enables two HIPSPV driver tests by removing blanket XFAIL markers.
Changes:
- Extract SYCL pipeline steps into
sycl::runSYCLOffloadingPipeline,sycl::postLinkProcessModule, and related helpers (includingcompileSYCLDevice). - Revert/refactor the generic
linkDevicepath and relocatecontainerizeRawImage/writeOffloadFileto align with upstream structure. - Remove
// XFAIL: *from HIPSPV toolchain/linking driver tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp | Refactors SYCL offloading into helper functions and adjusts device linking/wrapping flow. |
| clang/test/Driver/hipspv-toolchain.hip | Removes global XFAIL to re-enable the test. |
| clang/test/Driver/hipspv-link-static-library.hip | Removes global XFAIL to re-enable the test. |
remove subtle bug in sycl::wrapSYCLBinariesFromFile when Compile/Link options are moved out from the incoming images. rename function compileSYCLDevice to invokeBackendForSYCLDevice and make it accept only one file. rename function compileAndBundle to compileDeviceAndBundleNVPTXAndAMDGCN
|
@YuriPlyakhin @mdtoguchi @srividya-sundaram friendly ping. |
| // RUN: %t.without_triple.o -o %t.out 2>&1 --linker-path="/usr/bin/ld" | FileCheck %s --check-prefix=CHECK-ERROR-WITH-NO-TRIPLE | ||
|
|
||
| // CHECK-ERROR-WITH-NO-TRIPLE: linking is not supported | ||
| // CHECK-ERROR-WITH-NO-TRIPLE: can't compile a SYCL device code. target is unknown |
There was a problem hiding this comment.
not related to this PR, but a note for future PRs: this test should move to clang/test/OffloadTools/clang-linker-wrapper
There was a problem hiding this comment.
Good idea. We should move this part soon.
| // RUN: --bitcode-library=spir64_gen-unknown-unknown=%t1.devicelib.bc \ | ||
| // RUN: %t.wrong_spirv_subarch.o -o %t.out 2>&1 --linker-path="/usr/bin/ld" | FileCheck %s --check-prefix=CHECK-ERROR-WITH-WRONG-SPIRV-SUBARCH | ||
|
|
||
| // CHECK-ERROR-WITH-WRONG-SPIRV-SUBARCH: SYCL device compilation is not supported for the target: spirv64v1.0 No newline at end of file |
There was a problem hiding this comment.
nit: please restore newline at end of file
|
@intel/llvm-gatekeepers Could we please merge that? Failing tests are tracked here: #22295 |
|
These failures are unrelated to this patch? |
|
Yeah, they only fail in new offload model testing which is disabled by default and only run in the nightly, and the nightly is already seeing these failures |
Extract SYCL offloading steps into named functions within the sycl namespace: runSYCLOffloadingPipeline, postLinkProcessModule, compileAndBundle and compileSYCLDevice.
SYCL modifications of linkDevice function has been extracted to the dedicated function - compileSYCLDevice.
Revert linkDevice function to its upstream state. Side effect of this revert: hipspv tests get resolved. Related tracker: CMPLRLLVM-73247
generic::clang declaration is reverted to its upstream variant. IsSYCLKind is replaced with ActiveOffloadKindMask.
Functions containerizeRawImage and writeOffloadFile are moved so that their location corresponds to the upstream version.