Skip to content

Conversation

@cperkinsintel
Copy link
Contributor

For SYCL language support, the original POC wrote files to disk and invoked the clang compiler directly. This is suboptimal for various reasons. The SYCL_JIT compiler followed and is much faster. It is now supported on Windows as well, so there is no reason to have both.
In this PR we are removing the invoking version of kernel_compiler and making the SYCL_JIT one the default for supporting the source_language::sycl option.

@cperkinsintel cperkinsintel marked this pull request as ready for review November 8, 2024 22:33
@cperkinsintel cperkinsintel requested a review from a team as a code owner November 8, 2024 22:33
@cperkinsintel
Copy link
Contributor Author

precommit failures due to infra issue

Copy link
Contributor

@maarquitos14 maarquitos14 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep this (simpler) test for now, and just change it to use the sycl language?

The new approach doesn't handle the kernel_compiler_sycl.cpp test yet for a couple of unimplemented features (ESIMD, checking warnings, etc.). I believe the test failure there is currently hidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm demoting this PR back to "draft". That hidden test pass/fail made me not realize that the sycl_jit might not be quite ready yet to replace

@cperkinsintel cperkinsintel marked this pull request as draft November 12, 2024 20:00
@cperkinsintel cperkinsintel deleted the cperkins-remove-invoking-kernel_compiler branch March 27, 2025 00:07
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