Skip to content

[SYCL] Align get_info<info::device::version>() with the SYCL spec #2507

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 12, 2020
Merged

[SYCL] Align get_info<info::device::version>() with the SYCL spec #2507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e39575e
[SYCL] - Align get_info<info::device::version>() with the SYCL spec
DenisBakhvalov Sep 21, 2020
8d4876a
Fixed review comments
DenisBakhvalov Sep 22, 2020
7c6b856
Revert "Fixed review comments"
DenisBakhvalov Sep 22, 2020
db2b321
Fixed review comments
DenisBakhvalov Sep 22, 2020
1b40b9f
Run clang-format on sycl/test/basic_tests/parallel_for_range.cpp
DenisBakhvalov Sep 22, 2020
da074db
Added more tests for get_info<info::device::version>()
DenisBakhvalov Sep 22, 2020
2da0428
Fixed basic_tests/parallel_for_range.cpp
DenisBakhvalov Sep 23, 2020
b66ec6e
Updated sub_group/broadcast tests
DenisBakhvalov Sep 23, 2020
5bd0760
Fixed review comments
DenisBakhvalov Sep 23, 2020
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
36 changes: 34 additions & 2 deletions sycl/source/detail/device_info.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,9 @@ template <info::device param> struct get_device_info<platform, param> {
}
};

// Specialization for string return type, variable return size
template <info::device param> struct get_device_info<string_class, param> {
// Helper struct to allow using the specialization of get_device_info
// for string return type in other specializations.
template <info::device param> struct get_device_info_string {
static string_class get(RT::PiDevice dev, const plugin &Plugin) {
size_t resultSize;
Plugin.call<PiApiKind::piDeviceGetInfo>(
Expand All @@ -140,6 +141,13 @@ template <info::device param> struct get_device_info<string_class, param> {
}
};

// Specialization for string return type, variable return size
template <info::device param> struct get_device_info<string_class, param> {
static string_class get(RT::PiDevice dev, const plugin &Plugin) {
return get_device_info_string<param>::get(dev, Plugin);
}
};

// Specialization for parent device
template <typename T> struct get_device_info<T, info::device::parent_device> {
static T get(RT::PiDevice dev, const plugin &Plugin);
Expand Down Expand Up @@ -176,6 +184,30 @@ struct get_device_info<vector_class<info::fp_config>, param> {
}
};

// Specialization for OpenCL version, splits the string returned by OpenCL
template <> struct get_device_info<string_class, info::device::version> {
static string_class get(RT::PiDevice dev, const plugin &Plugin) {
string_class result =
get_device_info_string<info::device::version>::get(dev, Plugin);

// Extract OpenCL version from the returned string.
// For example, for the string "OpenCL 2.1 (Build 0)"
// return '2.1'.
auto dotPos = result.find('.');
if (dotPos == std::string::npos)
return result;

auto leftPos = result.rfind(' ', dotPos);
if (leftPos == std::string::npos)
leftPos = 0;
else
leftPos++;

auto rightPos = result.find(' ', dotPos);
return result.substr(leftPos, rightPos - leftPos);
Copy link
Contributor

Choose a reason for hiding this comment

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

the function is not robust. If version string contains "bla X.Y.Z bla" it will report X.Y.Z which also violates spec.

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, I know. It will also violate the spec if the plugin call returns the string without a dot, like 'XYZ'. What we should do in this case? Throwing an exception seems too strict, isn't it?

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 make sure that if format of the version returned by BE does not fit our requirements (e.g. no dot, X.Y.Z, X.Y at the end of string) the user friendly string will be displayed (e.g. the whole string returned by BE, X.Y(.Z is ignored), X.Y).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. This is what I'm doing if the string doesn't have the dot. E.g. For string "bla XdotY bla" I return "bla XdotY bla".
I'm not sure how you define "user friendly string".

}
};

// Specialization for single_fp_config, no type support check required
template <>
struct get_device_info<vector_class<info::fp_config>,
Expand Down
4 changes: 2 additions & 2 deletions sycl/source/detail/error_handling/enqueue_kernel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ bool handleInvalidWorkGroupSize(const device_impl &DeviceImpl, pi_kernel Kernel,
if (Platform.get_backend() == cl::sycl::backend::opencl) {
string_class VersionString = DeviceImpl.get_info<info::device::version>();
IsOpenCL = true;
IsOpenCLV1x = (VersionString.find("OpenCL 1.") == 0);
IsOpenCLV20 = (VersionString.find("OpenCL 2.0") == 0);
IsOpenCLV1x = (VersionString.find("1.") == 0);
IsOpenCLV20 = (VersionString.find("2.0") == 0);
}

size_t CompileWGSize[3] = {0};
Expand Down
37 changes: 37 additions & 0 deletions sycl/test/basic_tests/info_ocl_version.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out
// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
// RUN: env %CPU_RUN_PLACEHOLDER %t.out
// RUN: env %GPU_RUN_PLACEHOLDER %t.out
// RUN: env %ACC_RUN_PLACEHOLDER %t.out

//==--------info_ocl_version.cpp - SYCL objects get_info() test ------------==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include <CL/sycl.hpp>
#include <iostream>
#include <regex>
#include <string>

using namespace cl::sycl;

// This test checks that cl::sycl::info::device::version
// is returned in a form: <major_version>.<minor_version>

int main() {
default_selector selector;
device dev(selector.select_device());
auto ocl_version = dev.get_info<info::device::version>();
std::cout << ocl_version << std::endl;
const std::regex oclVersionRegex("[0-9]\\.[0-9]");
if (!std::regex_match(ocl_version, oclVersionRegex)) {
std::cout << "Failed" << std::endl;
return 1;
}
std::cout << "Passed" << std::endl;
return 0;
}
18 changes: 10 additions & 8 deletions sycl/test/basic_tests/parallel_for_range.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,18 @@ int main() {
string_class DeviceVendorName = D.get_info<info::device::vendor>();
auto DeviceType = D.get_info<info::device::device_type>();

const bool OCLBackend = D.get_platform().get_backend() == backend::opencl;
string_class OCLVersionStr = D.get_info<info::device::version>();
const bool OCLBackend = (OCLVersionStr.find("OpenCL") != string_class::npos);
assert((!OCLBackend || (OCLVersionStr.size() >= 10)) &&
"Unexpected device version string"); // strlen("OpenCL X.Y")
const char *OCLVersion = &OCLVersionStr[7]; // strlen("OpenCL ")
assert((OCLVersionStr.size() == 3) && "Unexpected device version string");
assert(OCLVersionStr.find(".") != string_class::npos &&
"Unexpected device version string");
const char OCLVersionMajor = OCLVersionStr[0];
const char OCLVersionMinor = OCLVersionStr[2];

// reqd_work_group_size is OpenCL specific.
if (OCLBackend) {
if (OCLVersion[0] == '1' ||
(OCLVersion[0] == '2' && OCLVersion[2] == '0')) {
if (OCLVersionMajor == '1' ||
(OCLVersionMajor == '2' && OCLVersionMinor == '0')) {
// parallel_for, (16, 16, 16) global, (8, 8, 8) local, reqd_wg_size(4, 4,
// 4)
// -> fail
Expand Down Expand Up @@ -143,7 +145,7 @@ int main() {
}
} // if (OCLBackend)

if (!OCLBackend || (OCLVersion[0] == '1')) {
if (!OCLBackend || (OCLVersionMajor == '1')) {
// OpenCL 1.x or non-OpenCL backends which behave like OpenCl 1.2 in SYCL.

// CL_INVALID_WORK_GROUP_SIZE if local_work_size is specified and
Expand Down Expand Up @@ -346,7 +348,7 @@ int main() {
<< std::endl;
return 1;
}
} else if (OCLBackend && (OCLVersion[0] == '2')) {
} else if (OCLBackend && (OCLVersionMajor == '2')) {
// OpenCL 2.x

// OpenCL 2.x:
Expand Down
4 changes: 2 additions & 2 deletions sycl/test/plugins/sycl-ls-gpu-opencl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// RUN: env SYCL_BE=PI_OPENCL sycl-ls --verbose >%t.opencl.out
// RUN: FileCheck %s --check-prefixes=CHECK-GPU-BUILTIN,CHECK-GPU-CUSTOM --input-file %t.opencl.out

// CHECK-GPU-BUILTIN: gpu_selector(){{.*}}GPU : OpenCL
// CHECK-GPU-CUSTOM: custom_selector(gpu){{.*}}GPU : OpenCL
// CHECK-GPU-BUILTIN: gpu_selector(){{.*}}GPU : {{[0-9]\.[0-9]}}
// CHECK-GPU-CUSTOM: custom_selector(gpu){{.*}}GPU : {{[0-9]\.[0-9]}}

//==-- sycl-ls-gpu-opencl.cpp - SYCL test for discovered/selected devices -===//
//
Expand Down
1 change: 0 additions & 1 deletion sycl/test/sub_group/broadcast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// XFAIL: cpu
// UNSUPPORTED: cuda
// CUDA compilation and runtime do not yet support sub-groups.

Expand Down
1 change: 0 additions & 1 deletion sycl/test/sub_group/broadcast_fp64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// XFAIL: cpu
// UNSUPPORTED: cuda
// CUDA compilation and runtime do not yet support sub-groups.

Expand Down