Skip to content

[SYCL][Driver] Validate the Intel GPU names of '--offload-arch' - #23060

Open
KornevNikita wants to merge 7 commits into
intel:syclfrom
KornevNikita:offload-arch-validate
Open

[SYCL][Driver] Validate the Intel GPU names of '--offload-arch'#23060
KornevNikita wants to merge 7 commits into
intel:syclfrom
KornevNikita:offload-arch-validate

Conversation

@KornevNikita

Copy link
Copy Markdown
Contributor

Record the IGCA level of every device in IntelGPUArch.def, so that the
table lists both names a device answers to, and add the names that cover
more than one release (xe-dg2, xe-mtl, xe-bmg, xe-ptl) with a sentinel
GMDID of zero. Move the table to clang/include/clang/Basic/ so that the
driver and the offload-arch tool can share it.

The driver now accepts, and validates against that table, the three
forms of a name the GPU driver uses: the name of an architecture
('xe-lnl-m'), the IGCA level shared by a group of architectures
('igca_40r'), and the numeric form the offload-arch tool prints for an
architecture this build has no name for ('xe_20.4.5'). Of the numeric
form only the architecture and the release are validated, as every
stepping of an architecture shares one name. Anything else is rejected
with the existing 'unsupported offload gpu architecture' diagnostic.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

KornevNikita and others added 5 commits August 28, 2026 14:45
…ames

The output of this utility is expected to be a list of names that are legal
--offload-arch parameters, but for Intel GPUs it printed the name of the
device, e.g. "Intel(R) Data Center GPU Max 1100", which is not one.

Query the GMDID of each device with zeDeviceGetProperties and the device IP
version extension, and translate its architecture and release components into
an architecture name using the table in IntelGPUArch.def, which is meant to be
generated from the data published by the GPU driver.  A device that has no
entry in the table is named after all three components of its GMDID, e.g.
xe_40.11.0, so that a GPU newer than the compiler is still usable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit a7e6682f197f7603915e5de75bc17683f8be6455)
Record the IGCA level of every device in IntelGPUArch.def, so that the
table lists both names a device answers to, and add the names that cover
more than one release (xe-dg2, xe-mtl, xe-bmg, xe-ptl) with a sentinel
GMDID of zero. Move the table to clang/include/clang/Basic/ so that the
driver and the offload-arch tool can share it.

The driver now accepts, and validates against that table, the three
forms of a name the GPU driver uses: the name of an architecture
('xe-lnl-m'), the IGCA level shared by a group of architectures
('igca_40r'), and the numeric form the offload-arch tool prints for an
architecture this build has no name for ('xe_20.4.5'). Of the numeric
form only the architecture and the release are validated, as every
stepping of an architecture shares one name. Anything else is rejected
with the existing 'unsupported offload gpu architecture' diagnostic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts:
#	clang/tools/offload-arch/LevelZeroArch.cpp
#	clang/unittests/offload-arch/OffloadArchTest.cpp

@tahonermann tahonermann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good. I added some comments regarding asserts and diagnostics.

Comment thread clang/include/clang/Basic/OffloadArch.h Outdated
Comment on lines +168 to +171
// Only valid when isIntelCPU(), or when isIntelGPU() and !isIntelXeGPU().
IntelArch intelKind() const { return static_cast<IntelArch>(Kind); }
// Only valid when isIntelXeGPU(); opaque outside of OffloadArch.cpp.
uint32_t intelXeKind() const { return Kind; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to assert the preconditions?

Suggested change
// Only valid when isIntelCPU(), or when isIntelGPU() and !isIntelXeGPU().
IntelArch intelKind() const { return static_cast<IntelArch>(Kind); }
// Only valid when isIntelXeGPU(); opaque outside of OffloadArch.cpp.
uint32_t intelXeKind() const { return Kind; }
// Only valid when isIntelCPU(), or when isIntelGPU() and !isIntelXeGPU().
IntelArch intelKind() const {
assert(isIntelCPU() || (isIntelGPU() && !isIntelXeGPU()), "Intel CPU or non-Xe GPU required");
return static_cast<IntelArch>(Kind);
}
// Only valid when isIntelXeGPU(); opaque outside of OffloadArch.cpp.
uint32_t intelXeKind() const {
assert(isIntelXeGPU(), "Intel Xe GPU required");
return Kind;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added asserts d992cfc

Comment thread clang/include/clang/Basic/OffloadArch.h Outdated
Comment on lines +134 to +136
static constexpr OffloadArch getIntelXeGPU(uint32_t Kind) {
return {TargetArch::IntelXeGPU, Kind};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be asserted that Kind matches a Xe GPU?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the function - d992cfc

Comment thread clang/lib/Basic/OffloadArch.cpp

@tahonermann tahonermann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KornevNikita, looks good to me. I added one comment regarding a possible group architecture addition, but I understand if such decisions aren't ours to make.

Comment on lines +40 to +45
INTEL_GPU_ARCH(35, 10, "xe-nvl-p", "igca_60r")
INTEL_GPU_ARCH(30, 5, "xe-nvl-u", "igca_60r")
INTEL_GPU_ARCH(30, 5, "xe-nvl-h", "igca_60r")
INTEL_GPU_ARCH(30, 4, "xe-nvl-s", "igca_60r")
INTEL_GPU_ARCH(30, 4, "xe-nvl-hx", "igca_60r")
INTEL_GPU_ARCH(30, 4, "xe-nvl-ul", "igca_60r")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all have a consistent name prefix and map to the same IGCA level. Should there not be a "xe-nvl" group name for these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC not all nvc-* devices share the same device code, e.g. xe-nvl-p is different from the rest of xe-nvl-*.

Actually xe-ptl covers all xe-nvl-u*, xe-nvl-h*, xe-nvl-s, xe-wcl and xe-ptl-* targets (but not xe-nvl-p), see https://github.com/intel/llvm/blob/sycl/sycl/doc/UsersManual.md.

And there are also other group names like xe-dg2 etc.

@KornevNikita

Copy link
Copy Markdown
Contributor Author

@srividya-sundaram could you please take a look?

@tahonermann

Copy link
Copy Markdown
Contributor

@KornevNikita, what is the plan for pushing these changes to upstream. We'll need them there to work on the frontend portions of the IGCA support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants