Repository navigation
[Clang][OffloadBundler] Match the target ID of SYCL bundle entry IDs - #23491
Conversation
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.
iclsrc
left a comment
iclsrc
left a comment
There was a problem hiding this comment.
I found no critical issues. The PR teaches OffloadTargetInfo to parse SYCL bundle entry IDs of the form sycl-<arch>-<vendor>-<os>-<targetid>. It also adds an isCodeObjectCompatible rule that treats SYCL code objects with no target ID as compatible with any target ID of the same triple. The parsing change is narrow: it requires 5 components, an unknown environment, and a recognized offload arch as the last component. The 4-component and 6-component forms are unaffected. I have one test-coverage comment below.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open findings
What changed in this PR
Fixes clang-offload-bundler target matching for SYCL bundle entry IDs by treating the final component (e.g. pvc) as a SYCL target ID rather than the triple environment, ensuring the correct device code is selected per architecture.
Changes:
- Update
OffloadTargetInfoparsing to recognize 3-component SYCL triples followed by a target ID. - Relax SYCL compatibility rules so bundles without a target ID still match bundles/targets of the same triple.
- Add a regression test covering multi-architecture SYCL (including NVPTX and “gen_image”) unbundling behavior.
| File | Description |
|---|---|
| clang/test/OffloadTools/clang-offload-bundler/sycl-target-id.c | Adds coverage to ensure each SYCL target ID unbundles to the correct code object and preserves compatibility for “no target ID” bundles. |
| clang/lib/Driver/OffloadBundler.cpp | Adjusts parsing of SYCL bundle IDs and compatibility logic to use target ID matching instead of misinterpreting it as triple environment. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Only treat 5-component bundle IDs as triple plus target ID for SYCL.
Thanks for the thorough review! |
|
CI failures are unrelated. Merging |
The bundler, runtime and sanitizer library fixes have landed in sycl (intel#23491, intel#23493, intel#23494), so their copies are dropped here. The linker wrapper fix is updated to the version in intel#23492.
Fixes #23487.
With several
intel_gpu_*(ornvidia_gpu_*) targets and separate compile and link,clang-offload-bundlerreturned the first architecture's device code for every architecture, because the architecture in SYCL bundle IDs likesycl-spir64_gen-unknown-unknown-pvcwas parsed as the triple environment.The architecture is now parsed as the target ID. A SYCL bundle without an architecture still matches any architecture of the same triple, so existing objects keep working.
Split out of #23441, whose tests need it.