[SYCL][Doc] Add kernel_function lambda wrapper #17633

Pennycook · 2025-03-25T11:30:32Z

We previously removed the ability to pass properties directly to functions like single_task, parallel_for, etc, in favor of requiring them to be attached to the kernel.

By introducing a new kernel_function wrapper that attaches properties to a kernel function, we remove the need for (most) developers to write such wrappers themselves.

We previously removed the ability to pass properties directly to functions like single_task, parallel_for, etc, in favor of requiring them to be attached to the kernel. By introducing a new kernel_function wrapper that attaches properties to a kernel function, we remove the need for (most) developers to write such wrappers themselves. Signed-off-by: John Pennycook <john.pennycook@intel.com>

While working on the kernel_function definition, I noticed that some text referred to "previous sections" that no longer exist. I tweaked the wording here to reflect that properties must always be attached to the kernel type. Signed-off-by: John Pennycook <john.pennycook@intel.com>

Pennycook · 2025-03-25T11:36:28Z

A few notes that may help with review, below.

This was my prototype implementation:

template <typename Function, typename Properties = syclx::empty_properties_t>
struct kernel_function {

    kernel_function(Function f, Properties p = syclx::properties{}) : f(f) {}

    template <typename... Args>
    std::enable_if_t<std::is_invocable_r_v<void, Function, Args...>>
    /*void*/ operator()(Args... args) const {
        f(std::forward<Args>(args)...);
    }

    auto get(syclx::properties_tag) {
        return Properties{};
    }

    private:
    const Function f;

};

I had planned to use is_nothrow_invocable, but that only works if the user specifies noexcept on their lambda (which nobody does). That's why the constraint only says "...invocable with Args" and not "...invocable with Args without throwing an exception".

One templated call operator seemed cleaner to me (and simpler to implement) than defining separate overloads for id<1>, id<2>, id<3>, item<1>, ... and so on. I'd be willing to switch to the separate overloads, but I think we should only do it if that would change the behavior in some observable way.

aelovikov-intel · 2025-03-25T14:04:58Z

sycl/doc/extensions/experimental/sycl_ext_oneapi_kernel_properties.asciidoc

+  auto get(syclx::properties_tag) {
+    return Properties{};
+  }


IMO, we should be storing properties object, in case it will contain runtime properties in future.

I think we also need to have two version of this - one static constexpr if Properties is std::empty_v, similarly to how properties' get_property is implemented.

IMO, we should be storing properties object, in case it will contain runtime properties in future.

The reason I didn't do this is that anything stored in the function object has to be transferred to the device as one of the kernel arguments. I think it would only make sense to allow this if the run-time properties were intended to be consumed within the kernel.

We'd also have to add an explicit specialization for the case where the property list is empty or contains only compile-time properties, to avoid transferring 1 byte unnecessarily. But perhaps this is a quality of implementation thing.

I think we also need to have two version of this - one static constexpr if Properties is std::empty_v, similarly to how properties' get_property is implemented.

Can you say more about this? Why would we only want it to be static constexpr in this case? Could we make it static constexpr if it doesn't contain any run-time properties, instead?

I think we also need to have two version of this - one static constexpr if Properties is std::empty_v, similarly to how properties' get_property is implemented.

Can you say more about this? Why would we only want it to be static constexpr in this case? Could we make it static constexpr if it doesn't contain any run-time properties, instead?

std::is_empty is exactly the check for compile-time-only (no run-time), see

llvm/sycl/include/sycl/ext/oneapi/properties/properties.hpp

Lines 256 to 273 in 5c5954e

// Compile-time property.

template <typename property_key_t>

static constexpr auto

get_property() -> std::enable_if_t<std::is_empty_v<prop_t<property_key_t>>,

prop_t<property_key_t>> {

return prop_t<property_key_t>{};

}

// Runtime property.

// Extra operand to make MSVC happy as it complains otherwise:

// https://godbolt.org/z/WGqdqrejj

template <typename property_key_t>

constexpr auto get_property(int = 0) const

-> std::enable_if_t<!std::is_empty_v<prop_t<property_key_t>>,

prop_t<property_key_t>> {

return get_property_impl(detail::property_key_tag<property_key_t>{});

}

};

I'm not sure if we have dedicated traits in the properties extension though. Maybe we can workaround that by using plain English in constraints/requirements?

But perhaps this is a quality of implementation thing.

Maybe we could protected inherit from the properties to do that, but I agree about QoI.

This does sound like an issue in the "base" part of the extension though. IMO, having a "getter" for the properties without one for the actual kernel is the root cause of this issue.

I've tried to address this in be618af; I've removed the implementation details from the synopsis, so that the implementation is no longer normative, and tried to describe the constraints we want.

The synopsis no longer shows what members are stored inside the kernel_function object, so implementations have the freedom to do whatever they want to meet the specified behavior.

aelovikov-intel · 2025-03-25T14:17:06Z

sycl/doc/extensions/experimental/sycl_ext_oneapi_kernel_properties.asciidoc

+template <typename Function, typename Properties = empty_properties_t>
+struct kernel_function {
+
+  kernel_function(Function f, Properties p = syclx::properties{});


Suggested change

kernel_function(Function f, Properties p = syclx::properties{});

kernel_function(Function &&f, Properties p = syclx::properties{});

And then std::move(f) inside implementation. That somewhat limits the applicability, but I'd rather start with that and extend later if needed.

Added in 3d580ff.

Okay, adding && to my prototype actually breaks it... Whether I use const && or &&, I get an error about expecting an rvalue. I thought && would accept both, but it seems not to work.

We want to support both uses below. What syntax do we need?

auto lambda = [=]() {}; auto kernel = syclx::kernel_function(lambda); // lambda is an l-value auto kernel = syclx::kernel_function([=]() {}); // lambda is an r-value

https://godbolt.org/z/fK36njGse works, but I don't know if that's the correct/idiomatic way. Otherwise, two overloads work too.

aelovikov-intel · 2025-03-25T14:17:26Z

sycl/doc/extensions/experimental/sycl_ext_oneapi_kernel_properties.asciidoc

+
+  // Available only if Function is invocable with Args
+  template <typename... Args>
+  void operator()(Args... args) const {


Suggested change

void operator()(Args... args) const {

void operator()(Args&& ...args) const {

I'm not sure about const too.

Added in 3d580ff.

Replace this with a detailed description of what the functions do. This is intended to permit implementations to specialize when properties is empty, to avoid transferring properties objects unnecessarily. Signed-off-by: John Pennycook <john.pennycook@intel.com>

aelovikov-intel · 2025-03-25T15:06:11Z

sycl/doc/extensions/experimental/sycl_ext_oneapi_kernel_properties.asciidoc

-  void operator()(Args... args) const {
-      f(std::forward<Args>(args)...);
-  }
+  void operator()(Args... args) const;


Suggested change

void operator()(Args... args) const;

void operator()(Args&& ...args) const;

is still needed.

Is this the same as the other suggestion? I'm a bit confused by how GitHub is rendering this.

Signed-off-by: John Pennycook <john.pennycook@intel.com>

syclx:: convention is only for examples. Signed-off-by: John Pennycook <john.pennycook@intel.com>

Pennycook added 2 commits March 25, 2025 11:20

Pennycook added the spec extension label Mar 25, 2025

Pennycook requested review from jbrodman, AlexeySachkov and steffenlarsen March 25, 2025 11:30

Pennycook requested a review from a team as a code owner March 25, 2025 11:30

Pennycook requested a review from aelovikov-intel March 25, 2025 11:31

aelovikov-intel reviewed Mar 25, 2025

View reviewed changes

Pennycook added 2 commits March 25, 2025 15:11

Add && to Function and Args

3d580ff

Signed-off-by: John Pennycook <john.pennycook@intel.com>

Remove syclx:: from synopsis and definitions

160fff7

syclx:: convention is only for examples. Signed-off-by: John Pennycook <john.pennycook@intel.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL][Doc] Add kernel_function lambda wrapper #17633

[SYCL][Doc] Add kernel_function lambda wrapper #17633

Pennycook commented Mar 25, 2025

Pennycook commented Mar 25, 2025

aelovikov-intel Mar 25, 2025 •

edited

Loading

Pennycook Mar 25, 2025

aelovikov-intel Mar 25, 2025 •

edited

Loading

aelovikov-intel Mar 25, 2025 •

edited

Loading

Pennycook Mar 25, 2025

aelovikov-intel Mar 25, 2025

Pennycook Mar 25, 2025

Pennycook Mar 25, 2025

aelovikov-intel Mar 25, 2025

aelovikov-intel Mar 25, 2025 •

edited

Loading

Pennycook Mar 25, 2025

aelovikov-intel Mar 25, 2025

Pennycook Mar 25, 2025

	// Compile-time property.
	template <typename property_key_t>
	static constexpr auto
	get_property() -> std::enable_if_t<std::is_empty_v<prop_t<property_key_t>>,
	prop_t<property_key_t>> {
	return prop_t<property_key_t>{};
	}

	// Runtime property.
	// Extra operand to make MSVC happy as it complains otherwise:
	// https://godbolt.org/z/WGqdqrejj
	template <typename property_key_t>
	constexpr auto get_property(int = 0) const
	-> std::enable_if_t<!std::is_empty_v<prop_t<property_key_t>>,
	prop_t<property_key_t>> {
	return get_property_impl(detail::property_key_tag<property_key_t>{});
	}
	};

	kernel_function(Function f, Properties p = syclx::properties{});
	kernel_function(Function &&f, Properties p = syclx::properties{});

	void operator()(Args... args) const {
	void operator()(Args&& ...args) const {

	void operator()(Args... args) const;
	void operator()(Args&& ...args) const;

[SYCL][Doc] Add kernel_function lambda wrapper #17633

Are you sure you want to change the base?

[SYCL][Doc] Add kernel_function lambda wrapper #17633

Conversation

Pennycook commented Mar 25, 2025

Pennycook commented Mar 25, 2025

aelovikov-intel Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aelovikov-intel Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

aelovikov-intel Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aelovikov-intel Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aelovikov-intel Mar 25, 2025 •

edited

Loading

aelovikov-intel Mar 25, 2025 •

edited

Loading

aelovikov-intel Mar 25, 2025 •

edited

Loading

aelovikov-intel Mar 25, 2025 •

edited

Loading