-
Notifications
You must be signed in to change notification settings - Fork 773
[NFCI] Introduce <sycl/detail/core.hpp> include #12890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Could you run something like:
and compare sycl.deps vs core.deps? From my experiments in the past, I noticed that just queue.hpp implicitly includes ~70% of SYCL headers. I would really want to document core.hpp as oneAPI, extension so it can be used by the users (not just tests). |
234f07a
to
90bbe9a
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whilst I support activities to speed up our tests (see my previous attempts in #7438), I have a concern about modifying E2E tests: they now can't be used as examples of SYCL usage, because they all contain non-standard undocumented functionality which can easily confuse our users.
I don't see any documentation being updated:
- it will be a bit harder (i.e. less obvious) for someone who haven't seen the patch to figure out what's going on
- we will have to explain this separately to everyone who is asking questions
- new tests will still be added with
sycl/sycl.hpp
unless someone thoroughly reviews all patches
I'm afraid this is a chicken and an egg problem. We can't make a meaningful documentation update (e.g. an official extension) until we've experimented enough and found out what we can reasonably achieve. On the other hand, I doubt anybody could work on this full-time and not being able to commit small incremental pieces would result in most of the time spent in rebases/merges slowing down any possible progress even further. I personally expect myself working on this not more than a few hours a week. I think the best we can have at the time is an informal discussion/direction alignment (like comments in this PR) and then anybody could keep making minor contribution until we have a better understanding of what works and what not. As a result, I think
is completely ok for the time being. Also, we already have some tests that only include individual headers and not the entire |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is great and should have been done from the beginning.
But I do think we should document it, even if just as an experimental extension. If it's useful to us, it's likely to be useful to other users.
The idea is to have a thinner version of `<sycl/sycl.hpp>` that allows to run basic tests and enables fine-grained includes in our e2e tests. Hopefully that can increase CI efficiency.
90bbe9a
to
c739683
Compare
@intel/syclcompat-lib-reviewers , @intel/dpcpp-esimd-reviewers , ping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No additional flags from ESIMD
…ore/*` Continuation of changes started in intel#12890.
Continuation of changes started in intel#12890.
Continuation of changes started in intel#12890.
Continuation of changes started in #12890.
The idea is to have a thinner version of
<sycl/sycl.hpp>
that allows to run basic tests and enables fine-grained includes in our e2e tests. Hopefully that can increase CI efficiency.