Skip to content

[SYCL] Add BFloat16 aspect #4266

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sycl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules")
include(AddSYCLExecutable)

set(SYCL_MAJOR_VERSION 5)
set(SYCL_MINOR_VERSION 3)
set(SYCL_MINOR_VERSION 4)
set(SYCL_PATCH_VERSION 0)
set(SYCL_DEV_ABI_VERSION 0)
if (SYCL_ADD_DEV_VERSION_POSTFIX)
Expand Down
1 change: 1 addition & 0 deletions sycl/include/CL/sycl/aspects.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ enum class aspect {
atomic64 = 28,
ext_intel_device_info_uuid = 29,
ext_oneapi_srgb = 30,
ext_intel_bf16_conversion = 31,
};

} // namespace sycl
Expand Down
1 change: 1 addition & 0 deletions sycl/include/CL/sycl/info/device_traits.def
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,4 @@ __SYCL_PARAM_TRAITS_SPEC(device, ext_intel_max_mem_bandwidth, pi_uint64)
__SYCL_PARAM_TRAITS_SPEC(device, ext_intel_mem_channel, bool)
__SYCL_PARAM_TRAITS_SPEC(device, ext_oneapi_srgb, bool)
__SYCL_PARAM_TRAITS_SPEC(device, ext_intel_device_info_uuid, detail::uuid_type)
__SYCL_PARAM_TRAITS_SPEC(device, ext_intel_bf16_conversion, bool)
1 change: 1 addition & 0 deletions sycl/include/CL/sycl/info/info_desc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ enum class device : cl_device_info {
atomic64 = PI_DEVICE_INFO_ATOMIC_64,
atomic_memory_order_capabilities =
PI_DEVICE_INFO_ATOMIC_MEMORY_ORDER_CAPABILITIES,
ext_intel_bf16_conversion,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to map it to a specific value in future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

};

enum class device_type : pi_uint64 {
Expand Down
22 changes: 22 additions & 0 deletions sycl/source/detail/device_info.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,23 @@ template <> struct get_device_info<device, info::device::parent_device> {
}
};

// Specialization for bfloat16
template <>
struct get_device_info<bool, info::device::ext_intel_bf16_conversion> {
static bool get(RT::PiDevice dev, const plugin &Plugin) {
// TODO: We claim, that all Intel GPU devices support bfloat16 conversion
// feature but we'd better have a low-level extension to query for support
// of the feature
Comment on lines +500 to +502
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any spec for the extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here it is #4237

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems #4237 is a spec for SYCL extension. Is there any spec for "a low-level extension"?

Could you please clarify why the temporary solution thru checking platform name is needed? Can we wait and implement the proper solution?

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 wait and implement the proper solution?

I'd like to have it merged soon. Is it ok if we merge it as is and update the implementation later, when we have the proper solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Coping my answer from below.

It seems #4237 is a spec for SYCL extension. Is there any spec for "a low-level extension"?

Nothing to share right now

Could you please clarify why the temporary solution thru checking platform name is needed? Can we wait and implement the proper solution?

So users could already write bfloat16 - sensitive code using aspect traits deciding for them which version of kernel should be enqueued: with or without bf16 conversion feature.

platform plt =
get_device_info<platform, info::device::platform>::get(dev, Plugin);
std::string platform_name = plt.get_info<info::platform::name>();
if (platform_name == "Intel(R) OpenCL HD Graphics")
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe, there should be a better way to verify the device is either of Intel's GPUs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now there is no low-level runtime specification for the feature, so it's the best, what came in my mind. Do you have any ideas?

return true;

return false;
}
};

// SYCL host device information

// Default template is disabled, all possible instantiations are
Expand Down Expand Up @@ -998,6 +1015,11 @@ inline bool get_device_info_host<info::device::ext_intel_mem_channel>() {
return false;
}

template <>
inline bool get_device_info_host<info::device::ext_intel_bf16_conversion>() {
return false;
}

cl_uint get_native_vector_width(size_t idx);

// USM
Expand Down
1 change: 1 addition & 0 deletions sycl/test/abi/sycl_symbols_linux.dump
Original file line number Diff line number Diff line change
Expand Up @@ -4173,6 +4173,7 @@ _ZNK2cl4sycl6device8get_infoILNS0_4info6deviceE65574EEENS3_12param_traitsIS4_XT_
_ZNK2cl4sycl6device8get_infoILNS0_4info6deviceE65575EEENS3_12param_traitsIS4_XT_EE11return_typeEv
_ZNK2cl4sycl6device8get_infoILNS0_4info6deviceE65808EEENS3_12param_traitsIS4_XT_EE11return_typeEv
_ZNK2cl4sycl6device8get_infoILNS0_4info6deviceE65809EEENS3_12param_traitsIS4_XT_EE11return_typeEv
_ZNK2cl4sycl6device8get_infoILNS0_4info6deviceE65810EEENS3_12param_traitsIS4_XT_EE11return_typeEv
_ZNK2cl4sycl6device9getNativeEv
_ZNK2cl4sycl6kernel11get_contextEv
_ZNK2cl4sycl6kernel11get_programEv
Expand Down
3 changes: 3 additions & 0 deletions sycl/test/on-device/basic_tests/aspects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ int main() {
if (plt.has(aspect::usm_system_allocations)) {
std::cout << " USM system allocations" << std::endl;
}
if (plt.has(aspect::ext_intel_bf16_conversion)) {
std::cout << " BFloat16 conversion extension" << std::endl;
}
}
std::cout << "Passed." << std::endl;
return 0;
Expand Down