Skip to content
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

Clarify backend and introduce "backend::none" #577

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Jul 1, 2024

Clarify exactly what we mean by "backend". I think it has always been our intention that the backend enumeration and the get_backend member function tell an application what sort of interoperation is supported by a SYCL object. For example, if a SYCL object returns backend::opencl from its get_backend function, that SYCL object supports interoperation as defined by the OpenCL backend interoperation specification.

This commit clarifies the spec to say that explicitly.

It also became clear that we need a backend::none enumerator to indicate that a SYCL object does not support any backend interoperation. For example, this would be useful if a vendor implements SYCL directly on hardware, without using a documented low-level offload API. Many of the changes in this commit merely remove sentences that imply that every SYCL object necessarily has some backend.

This commit also makes a minor change to the default event constructor. Previously, we required the backend for such an event to be the same as the backend for the default device. After some implementation experience, we decided that this is not practical. Requiring interoperation with a particular backend constrains the implementation too much. It also seems arbitrary to require the default-constructed event to have a particular backend. To address these concerns, this commit loosens the requirement, allowing an implementation to choose which backend (if any) the default-constructed event has.

Closes #564

gmlueck and others added 3 commits July 1, 2024 15:38
Clarify exactly what we mean by "backend".  I think it has always been
our intention that the `backend` enumeration and the `get_backend`
member function tell an application what sort of interoperation is
supported by a SYCL object.  For example, if a SYCL object returns
`backend::opencl` from its `get_backend` function, that SYCL object
supports interoperation as defined by the OpenCL backend interoperation
specification.

This commit clarifies the spec to say that explicitly.

It also became clear that we need a `backend::none` enumerator to
indicate that a SYCL object does not support any backend interoperation.
For example, this would be useful if a vendor implements SYCL directly
on hardware, without using a documented low-level offload API.  Many of
the changes in this commit merely remove sentences that imply that every
SYCL object necessarily has some backend.

This commit also makes a minor change to the default `event`
constructor.  Previously, we required the backend for such an event to
be the same as the backend for the default device.  After some
implementation experience, we decided that this is not practical.
Requiring interoperation with a particular backend constrains the
implementation too much.  It also seems arbitrary to require the
default-constructed event to have a particular backend.  To address
these concerns, this commit loosens the requirement, allowing an
implementation to choose which backend (if any) the default-constructed
event has.

Closes KhronosGroup#564
The previous wording placed a lot of emphasis on backends, defining two
different kinds of SYCL application. Since backends and backend
interoperability are optional, reducing this emphasis is desirable.
Additionally, removing this section from the specification simplifies it and
makes it easier to read.
Earlier commits reworded several sentences to remove references to
"SYCL general" and "SYCL generic" by referring explicitly to backends instead.
This commit completes that work by removing the remaining references to these
concepts.
adoc/chapters/architecture.adoc Show resolved Hide resolved
adoc/chapters/architecture.adoc Outdated Show resolved Hide resolved
@@ -15935,7 +15949,10 @@ The following member functions provide various queries for a <<kernel>>.
backend get_backend() const noexcept;
----

_Returns:_ The backend associated with this kernel.
_Returns:_ The backend (if any) that underlies this object.
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's a philosophical question, but is a backend none a backend or not? If it's then we can remove all the if any everywhere :) (Russell’s Paradox here I come)

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 specified backend::none as "not a backend". Here is the description from the section on the backend enumeration:

The enumerator value backend::none is used to identify a SYCL object that does not correspond to any backend.

In addition, the section "The SYCL backend model" describes backends this way:

A SYCL implementation may expose devices by layering on top of a heterogeneous API such as OpenCL or it may expose devices in some other way. [...] When a SYCL implementation layers on top of a heterogeneous API such as OpenCL, we refer to this as a SYCL backend.

Thus, when an implementation exposes devices without layering on top of a heterogeneous API, it is not using a backend.

Copy link
Contributor

@TApplencourt TApplencourt Jul 10, 2024

Choose a reason for hiding this comment

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

Would you happen to know the reason for such a choice for restricting backend to "heterogeneous API" ?

That a TBB or a OpenMP are not considered backends seems odd. It makes sense that they don't provide any interop, but I think the general usage of the word "backend" is more general.

For example, in the SimSYCL paper, they use the term "AdaptiveCpp (OMP backend)" (https://dl.acm.org/doi/fullHtml/10.1145/3648115.3648136 table 2) who make total sense to me, but are not technically correct.

In short, I don't understand the rationale behind this spec subtility. Every backend can decide if they provide or not some interopts for whatever SYCL function they want.

PS: This also led to a question about an MPI Implementation. As far as I know, MPI is not a "heterogeneous API" (just an API :) ), but I guess we still want to be able to give some interrupt handle.

Copy link
Contributor

@illuhad illuhad Jul 10, 2024

Choose a reason for hiding this comment

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

That a TBB or a OpenMP are not considered backends seems odd. It makes sense that they don't provide any interop, but I think the general usage of the word "backend" if more general.

Yes. I'm opposed to tying the notion of a "backend" to the ability of providing interop. Those two have been separate concepts so far in SYCL.

AdaptiveCpp treats OpenMP just like any other backend. I see no reason to change the backend model for "native CPU" approaches. From the AdaptiveCpp point of view, those are perfectly valid backends, and I would disagree with everyone who claims that they are not SYCL backends.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The term "heterogeneous API" is used in the SYCL specification even before this PR. As far as I know, there is no industry standard definition for this term. I was assuming that OMP would also be considered a "heterogeneous API".

I'm not opposed to using a different term if someone has a suggestion. Is there some better collective term for things like OpenCL, OpenMP, CUDA, etc.? Maybe "offload API"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend disconnecting "heterogeneous API" from any "backend" consideration. But maybe it is not the best place to discuss this.

My main point is that we tied too much of the SYCL spec to the "heterogeneous API" of the backend.
For example:

A function object that can execute on a device exposed by a SYCL backend API is called a SYCL kernel function.

That means that for an OpenMP pure CPU or Intel TBB, a lambda used in Q.submit is not a kernel function. That also means that we can have "devices" that are not part of any "backend"?

OMP for offloading I I agree, is "heterogenous"; for CPU only OpenMP I don't see why it's "heterogeneous". Same for MPI. Or for a pthread CPU implementation of SYCL.

I think all this "SYCL is build on top of a backend API" is a relic of OpenCL; we should clarify that it's just implementation details. SYCL provides a "heterogenous API" but doesn't need to one be implemented. The spec should be clear about it.

And I think the easiest way is to define loosely what is a backend; it's whatever the implementation wants it to be, and not tie it to any concept of "heterogenous API"

But yeah, maybe a far more broad conversation; sorry for the side-discussion.

@tomdeakin tomdeakin added the Agenda To be discussed during a SYCL committee meeting label Jul 10, 2024
@illuhad
Copy link
Contributor

illuhad commented Jul 10, 2024

I think it has always been our intention that the backend enumeration and the get_backend member function tell an application what sort of interoperation is supported by a SYCL object. For example, if a SYCL object returns backend::opencl from its get_backend function, that SYCL object supports interoperation as defined by the OpenCL backend interoperation specification.

I would be careful to tie the SYCL backend model too closely to the interop model. There may be other use cases for querying the backend, for example if you know that e.g. that a specific backend has certain performance characteristics.

I think I am opposed to adding backend::none. It feels like it fundamentally changes the SYCL backend model, which is a core design aspect of SYCL, just to fix some edge cases of the interop model.

I believe it would be clearer for SYCL if the backend values always referred to some valid backend. Interop cases that can not clearly be assigned to one backend like event should return std::vector<backend> (which could contain zero or multiple values) or std::optional<backend>.

See discussion here for details: #564

@tomdeakin
Copy link
Contributor

Next steps:

  1. PR for "Default constructed event should be associated with an implementation defined backend."
  2. More discussion on backend and interop

@TApplencourt
Copy link
Contributor

TApplencourt commented Jul 11, 2024

To clarify my point (sorry if I was unclear earlier), IMO we have two distinct types of "interop errors."

  • One is backend-specific. The backend implementer can decide what they implement and what to not implement and the restriction for their usage. For example, get device interopt for CPU only or whatever.
    This is a local decision made by the backend implementer. And should be part of backend-specification.

  • The second is a "cross-backend" error. For example, a default constructed event is not associated with any backend. The question is not about the "interopt" but about which backend this default event is bound to. This cannot be solved by any backend-spec, it need to be specified inside the core-sycl-spec (for example said it's UB :'0)

One is an "I Don't Know the Backend. Sorry, general SYCL problem" and the other is a "Not Implement Error for this backend. Contact your vendor.".
Using the return value of "backend" for those 2 errors, seems wrong to me. They are not solved in the same part of the spec. One is a general SYCL UB problem, and the other is an "implementation detail" of a specific backend-spec.

For me backend::none / backend::unspecified should be reserved for the "cross backend problem". The other non-implemented / non valid usage of interopt API should throw or whatever the interopt backend said. It's a get_native problem/contract. It's not a backend problem. The backend is know, it's just that you cannot use get_native.

( If we want to avoid the throw,, we should add a new API call, something like is_get_native_avalaible or whatever that people can use to check if they can use get_native or not.)

TLDR: get_backend behavior should be specified in the core-spec. get_native should be specified by the backend-spec.
Hope this clarify,

@gmlueck
Copy link
Contributor Author

gmlueck commented Jul 12, 2024

PR for "Default constructed event should be associated with an implementation defined backend."

I created a separate PR with just this change #582

Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

That sounds good.

Although the APIs in the SYCL generic programming model are defined according to
this specification and their version is indicated by the macro
Although most APIs in the SYCL programming model are defined according to this
specification and their version is indicated by the macro
Copy link
Member

Choose a reason for hiding this comment

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

Even if it was in the previous version, and their version looks weird to me.
Perhaps we can clarify the meaning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to clean this up, I would propose deleting this entire paragraph. I don't think it adds any clarity. I don't think the core spec needs to say anything about the versioning of the backend interoperation APIs. This can be left to each backend interoperation specification.

@gmlueck
Copy link
Contributor Author

gmlueck commented Jul 25, 2024

From WG meeting today:

  • Clarify that you cannot use backend enumerator as template parameter to traits.

@gmlueck gmlueck removed the Agenda To be discussed during a SYCL committee meeting label Jul 31, 2024
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.

Should we add "backend::none"?
6 participants