Skip to content

[SYCL] Improve error message when no compatible device image is found#21749

Open
kweronsx wants to merge 5 commits intosyclfrom
fix/improve-no-compatible-device-error
Open

[SYCL] Improve error message when no compatible device image is found#21749
kweronsx wants to merge 5 commits intosyclfrom
fix/improve-no-compatible-device-error

Conversation

@kweronsx
Copy link
Copy Markdown
Contributor

ProgramManager::getDeviceImage previously threw the same "No kernel named X was found" error for two distinct situations:

  • the kernel name is genuinely unknown (not registered at all), and
  • the kernel is known but no binary image in the fat binary targets the device being used at runtime.

The second case is typically caused by compiling for a different device architecture than the one executing the binary, which is a common user mistake. Conflating it with an unknown kernel name made the error misleading and hard to diagnose.

Introduce a KernelFound flag to distinguish the two paths and emit a dedicated message for the no-compatible-image case.

Comment on lines +1355 to +1356
std::string(KernelName) +
". The program may not have been compiled for this device.");
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.

Suggested change
std::string(KernelName) +
". The program may not have been compiled for this device.");
std::string(KernelName);

Nitpick: I'd prefer to keep exception messages on the shorter side and have them be just a basic description of the problem. The last sentence here is likely to be accurate, but I don't think it provides much value, so I would drop it. Might be a matter of personal taste though.

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.

I think you may be right, it seems to be somewhat of a matter of personal taste, but actually I agree. As the old saying goes: "when a program has nothing surprising, interesting or useful to say, it should say nothing".

@kweronsx kweronsx force-pushed the fix/improve-no-compatible-device-error branch from eaa5eb9 to 3019b95 Compare April 14, 2026 07:08
@bratpiorka bratpiorka requested a review from Maetveis April 14, 2026 08:49
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
Copy link
Copy Markdown
Contributor

@Maetveis Maetveis left a comment

Choose a reason for hiding this comment

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

LGTM, with a nit. Thank you!

ProgramManager::getDeviceImage previously threw the same "No kernel
named X was found" error for two distinct situations:
- the kernel name is genuinely unknown (not registered at all), and
- the kernel is known but no binary image in the fat binary targets
  the device being used at runtime.

The second case is typically caused by compiling for a different device
architecture than the one executing the binary, which is a common user
mistake. Conflating it with an unknown kernel name made the error
misleading and hard to diagnose.

Introduce a KernelFound flag to distinguish the two paths and emit a
dedicated message for the no-compatible-image case.
@kweronsx kweronsx force-pushed the fix/improve-no-compatible-device-error branch from 3019b95 to 45e1f6a Compare April 14, 2026 10:51
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
@github-actions
Copy link
Copy Markdown
Contributor

@intel/llvm-gatekeepers please consider merging

@kweronsx kweronsx marked this pull request as draft April 15, 2026 06:15
Add DeviceBinaryProperty::asStringView() to read string-valued properties
without reinterpret_cast or manually skipping the 8-byte size header. Use
it in getImageTargetLabel() and doesImageTargetMatchDevice() to replace the
previous asByteArray()/dropBytes(8)/reinterpret_cast pattern.

Extract the comma-separated available-target list building into a new
ProgramManager::getKernelTargetList() member function.

Update the error message wording from "has no image for the current device.
It was compiled for:" to "has no image for the selected device. Its
available images target:" — "current" was misleading since the API can be
called for an explicitly chosen device, and "compiled for" implied those
are the only targets the kernel was ever built for. Update the
NoKernelDevice unit test accordingly.
@kweronsx kweronsx marked this pull request as ready for review April 15, 2026 07:32
@kweronsx kweronsx requested a review from Maetveis April 15, 2026 07:32
Copy link
Copy Markdown
Contributor

@Maetveis Maetveis left a comment

Choose a reason for hiding this comment

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

LGTM, few more nits, sorry :)

Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
Comment thread sycl/source/detail/device_binary_image.cpp
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
@kweronsx kweronsx marked this pull request as draft April 15, 2026 07:55
Return std::string_view from getImageTargetLabel() to avoid a heap
allocation per image. Add an assert that it is never empty. Add the
type-mismatch assert from asCString() to asStringView() as well, since
the latter makes the same assumptions about Prop->Type.
@kweronsx kweronsx marked this pull request as ready for review April 15, 2026 08:54
@kweronsx kweronsx requested a review from Maetveis April 15, 2026 08:54
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
Comment thread sycl/source/detail/program_manager/program_manager.cpp Outdated
@kweronsx kweronsx marked this pull request as draft April 15, 2026 11:46
@kweronsx kweronsx marked this pull request as ready for review April 15, 2026 12:10
@kweronsx kweronsx requested a review from Maetveis April 15, 2026 12:10
Simplify getImageTargetLabel(): rely on the implicit const char* to
std::string_view conversion for the DeviceTargetSpec fallback, dropping
the now-unnecessary named variable and assert.

Rewrite getKernelTargetList() using a first-element-then-rest pattern:
initialise the result with the first target, then prepend ", " before
each subsequent one. This replaces the previous approach of appending a
trailing delimiter after every element and then trimming it, and removes
the associated assert on the string length.
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.

3 participants