Skip to content

Commit 73ff319

Browse files
[SYCL] Add size to detail::string_view (#18661)
This allows us to pass size of kernel name that is known in compile time, not compute the size in run time. --------- Signed-off-by: Alexandr Konovalov <[email protected]>
1 parent 101c8d8 commit 73ff319

File tree

15 files changed

+119
-40
lines changed

15 files changed

+119
-40
lines changed

sycl/include/sycl/detail/kernel_name_str_t.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
66
//
77
//===----------------------------------------------------------------------===//
8+
#pragma once
89

910
#include <sycl/detail/string.hpp>
1011
#include <sycl/detail/string_view.hpp>
@@ -23,6 +24,14 @@ using KernelNameStrRefT = const std::string &;
2324
using ABINeutralKernelNameStrT = detail::string;
2425
#endif
2526

27+
inline KernelNameStrT toKernelNameStrT(const ABINeutralKernelNameStrT &str) {
28+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
29+
return std::string_view(str);
30+
#else
31+
return str.data();
32+
#endif
33+
}
34+
2635
} // namespace detail
2736
} // namespace _V1
2837
} // namespace sycl

sycl/include/sycl/detail/string.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace detail {
1616

1717
// This class and detail::string_view class are intended to support
1818
// different ABIs between libsycl and the user program.
19-
// This class is not inteded to replace std::string for general purpose usage.
19+
// This class is not intended to replace std::string for general purpose usage.
2020
class string {
2121
char *str = nullptr;
2222

sycl/include/sycl/detail/string_view.hpp

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,33 +17,80 @@ namespace detail {
1717

1818
// This class and detail::string class are intended to support
1919
// different ABIs between libsycl and the user program.
20-
// This class is not inteded to replace std::string_view for general purpose
20+
// This class is not intended to replace std::string_view for general purpose
2121
// usage.
22+
2223
class string_view {
2324
const char *str = nullptr;
25+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
26+
size_t len = 0;
27+
#endif
2428

2529
public:
2630
string_view() noexcept = default;
2731
string_view(const string_view &strn) noexcept = default;
2832
string_view(string_view &&strn) noexcept = default;
29-
string_view(std::string_view strn) noexcept : str(strn.data()) {}
30-
string_view(const sycl::detail::string &strn) noexcept : str(strn.c_str()) {}
33+
string_view(std::string_view strn) noexcept
34+
: str(strn.data())
35+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
36+
,
37+
len(strn.size())
38+
#endif
39+
{
40+
}
41+
string_view(const sycl::detail::string &strn) noexcept
42+
: str(strn.c_str())
43+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
44+
,
45+
len(strlen(strn.c_str()))
46+
#endif
47+
{
48+
}
3149

3250
string_view &operator=(string_view &&strn) noexcept = default;
3351
string_view &operator=(const string_view &strn) noexcept = default;
3452

3553
string_view &operator=(std::string_view strn) noexcept {
3654
str = strn.data();
55+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
56+
len = strn.size();
57+
#endif
3758
return *this;
3859
}
3960

4061
string_view &operator=(const sycl::detail::string &strn) noexcept {
4162
str = strn.c_str();
63+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
64+
len = strlen(strn.c_str());
65+
#endif
4266
return *this;
4367
}
4468

4569
const char *data() const noexcept { return str ? str : ""; }
4670

71+
explicit operator std::string_view() const noexcept {
72+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
73+
return std::string_view(str, len);
74+
#else
75+
return std::string_view(str);
76+
#endif
77+
}
78+
79+
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
80+
friend bool operator==(string_view lhs, std::string_view rhs) noexcept {
81+
return rhs == std::string_view(lhs);
82+
}
83+
friend bool operator==(std::string_view lhs, string_view rhs) noexcept {
84+
return lhs == std::string_view(rhs);
85+
}
86+
87+
friend bool operator!=(string_view lhs, std::string_view rhs) noexcept {
88+
return rhs != std::string_view(lhs);
89+
}
90+
friend bool operator!=(std::string_view lhs, string_view rhs) noexcept {
91+
return lhs != std::string_view(rhs);
92+
}
93+
#else
4794
friend bool operator==(string_view lhs, std::string_view rhs) noexcept {
4895
return rhs == lhs.data();
4996
}
@@ -57,6 +104,7 @@ class string_view {
57104
friend bool operator!=(std::string_view lhs, string_view rhs) noexcept {
58105
return lhs != rhs.data();
59106
}
107+
#endif
60108
};
61109

62110
} // namespace detail

sycl/include/sycl/handler.hpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -537,11 +537,12 @@ class __SYCL_EXPORT handler {
537537
template <typename LambdaNameT> bool lambdaAndKernelHaveEqualName() {
538538
// TODO It is unclear a kernel and a lambda/functor must to be equal or not
539539
// for parallel_for with sycl::kernel and lambda/functor together
540-
// Now if they are equal we extract argumets from lambda/functor for the
540+
// Now if they are equal we extract arguments from lambda/functor for the
541541
// kernel. Else it is necessary use set_atg(s) for resolve the order and
542542
// values of arguments for the kernel.
543543
assert(MKernel && "MKernel is not initialized");
544-
const std::string LambdaName = detail::getKernelName<LambdaNameT>();
544+
constexpr std::string_view LambdaName =
545+
detail::getKernelName<LambdaNameT>();
545546
detail::ABINeutralKernelNameStrT KernelName = getKernelName();
546547
return KernelName == LambdaName;
547548
}
@@ -865,7 +866,9 @@ class __SYCL_EXPORT handler {
865866
&(detail::getKernelParamDesc<KernelName>),
866867
detail::isKernelESIMD<KernelName>(), HasSpecialCapt);
867868

868-
MKernelName = detail::getKernelName<KernelName>();
869+
constexpr std::string_view KernelNameStr =
870+
detail::getKernelName<KernelName>();
871+
MKernelName = KernelNameStr;
869872
} else {
870873
// In case w/o the integration header it is necessary to process
871874
// accessors from the list(which are associated with this handler) as
@@ -1325,8 +1328,9 @@ class __SYCL_EXPORT handler {
13251328
KernelWrapper<WrapAs::parallel_for, NameT, KernelType, TransformedArgType,
13261329
PropertiesT>::wrap(this, KernelFunc);
13271330
#ifndef __SYCL_DEVICE_ONLY__
1328-
verifyUsedKernelBundleInternal(
1329-
detail::string_view{detail::getKernelName<NameT>()});
1331+
constexpr std::string_view Name{detail::getKernelName<NameT>()};
1332+
1333+
verifyUsedKernelBundleInternal(detail::string_view{Name});
13301334
processProperties<detail::isKernelESIMD<NameT>(), PropertiesT>(Props);
13311335
detail::checkValueRange<Dims>(UserRange);
13321336
setNDRangeDescriptor(std::move(UserRange));

sycl/source/backend/opencl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ __SYCL_EXPORT bool has_extension(const sycl::platform &SyclPlatform,
3535
std::string ExtensionsString = urGetInfoString<UrApiKind::urPlatformGetInfo>(
3636
*getSyclObjImpl(SyclPlatform), UR_PLATFORM_INFO_EXTENSIONS);
3737

38-
return ExtensionsString.find(std::string_view{Extension.data()}) !=
38+
return ExtensionsString.find(std::string_view{Extension}) !=
3939
std::string::npos;
4040
}
4141

@@ -50,7 +50,7 @@ __SYCL_EXPORT bool has_extension(const sycl::device &SyclDevice,
5050
std::string ExtensionsString = urGetInfoString<UrApiKind::urDeviceGetInfo>(
5151
*getSyclObjImpl(SyclDevice), UR_DEVICE_INFO_EXTENSIONS);
5252

53-
return ExtensionsString.find(std::string_view{Extension.data()}) !=
53+
return ExtensionsString.find(std::string_view{Extension}) !=
5454
std::string::npos;
5555
}
5656
} // namespace detail

sycl/source/detail/device_image_impl.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ class device_image_impl {
853853
extractXsFlags(const std::vector<sycl::detail::string_view> &BuildOptions) {
854854
std::stringstream SS;
855855
for (sycl::detail::string_view Option : BuildOptions) {
856-
std::string_view OptionSV{Option.data()};
856+
std::string_view OptionSV{Option};
857857
auto Where = OptionSV.find("-Xs");
858858
if (Where != std::string_view::npos) {
859859
Where += 3;

sycl/source/detail/graph_impl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1904,7 +1904,7 @@ void modifiable_command_graph::end_recording(
19041904

19051905
void modifiable_command_graph::print_graph(sycl::detail::string_view pathstr,
19061906
bool verbose) const {
1907-
std::string path{pathstr.data()};
1907+
std::string path{std::string_view(pathstr)};
19081908
graph_impl::ReadLock Lock(impl->MMutex);
19091909
if (path.substr(path.find_last_of(".") + 1) == "dot") {
19101910
impl->printGraphAsDot(std::move(path), verbose);

sycl/source/detail/helpers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ markBufferAsInternal(const std::shared_ptr<buffer_impl> &BufImpl) {
3838
}
3939

4040
std::tuple<const RTDeviceBinaryImage *, ur_program_handle_t>
41-
retrieveKernelBinary(queue_impl &Queue, const char *KernelName,
41+
retrieveKernelBinary(queue_impl &Queue, KernelNameStrRefT KernelName,
4242
CGExecKernel *KernelCG) {
4343
device_impl &Dev = Queue.getDeviceImpl();
4444
bool isNvidia = Dev.getBackend() == backend::ext_oneapi_cuda;

sycl/source/detail/helpers.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include <sycl/detail/kernel_name_str_t.hpp>
10+
911
#include <ur_api.h>
1012

1113
#include <memory>
@@ -27,7 +29,7 @@ void waitEvents(std::vector<sycl::event> DepEvents);
2729
#endif
2830

2931
std::tuple<const RTDeviceBinaryImage *, ur_program_handle_t>
30-
retrieveKernelBinary(queue_impl &Queue, const char *KernelName,
32+
retrieveKernelBinary(queue_impl &Queue, KernelNameStrRefT KernelName,
3133
CGExecKernel *CGKernel = nullptr);
3234
} // namespace detail
3335
} // namespace _V1

sycl/source/detail/scheduler/commands.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3250,7 +3250,7 @@ ur_result_t ExecCGCommand::enqueueImpQueue() {
32503250
const RTDeviceBinaryImage *BinImage = nullptr;
32513251
if (detail::SYCLConfig<detail::SYCL_JIT_AMDGCN_PTX_KERNELS>::get()) {
32523252
std::tie(BinImage, std::ignore) =
3253-
retrieveKernelBinary(*MQueue, KernelName.data());
3253+
retrieveKernelBinary(*MQueue, KernelName);
32543254
assert(BinImage && "Failed to obtain a binary image.");
32553255
}
32563256
enqueueImpKernel(

sycl/source/device.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ template __SYCL_EXPORT std::vector<device> device::create_sub_devices<
118118
info::partition_property::ext_intel_partition_by_cslice>() const;
119119

120120
bool device::has_extension(detail::string_view ext_name) const {
121-
return impl->has_extension(ext_name.data());
121+
return impl->has_extension(std::string(std::string_view(ext_name)));
122122
}
123123

124124
template <typename Param>
@@ -292,7 +292,7 @@ bool device::ext_oneapi_supports_cl_c_feature(detail::string_view Feature) {
292292
return false;
293293

294294
return ext::oneapi::experimental::detail::OpenCLC_Feature_Available(
295-
Feature.data(), ipVersion);
295+
std::string(std::string_view(Feature)), ipVersion);
296296
}
297297

298298
bool device::ext_oneapi_supports_cl_c_version(
@@ -321,7 +321,7 @@ bool device::ext_oneapi_supports_cl_extension(
321321
return false;
322322

323323
return ext::oneapi::experimental::detail::OpenCLC_Supports_Extension(
324-
Name.data(), VersionPtr, ipVersion);
324+
std::string(std::string_view(Name)), VersionPtr, ipVersion);
325325
}
326326

327327
detail::string device::ext_oneapi_cl_profile_impl() const {

sycl/source/device_selector.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,8 @@ int accelerator_selector::operator()(const device &dev) const {
283283
namespace ext::oneapi {
284284

285285
filter_selector::filter_selector(sycl::detail::string_view Input)
286-
: impl(std::make_shared<detail::filter_selector_impl>(Input.data())) {}
286+
: impl(std::make_shared<detail::filter_selector_impl>(
287+
std::string(std::string_view(Input)))) {}
287288

288289
int filter_selector::operator()(const device &Dev) const {
289290
return impl->operator()(Dev);

sycl/source/handler.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -480,13 +480,13 @@ event handler::finalize() {
480480
!(MKernel && MKernel->isInterop()) &&
481481
(KernelBundleImpPtr->empty() ||
482482
KernelBundleImpPtr->hasSYCLOfflineImages()) &&
483-
!KernelBundleImpPtr->tryGetKernel(MKernelName.data(),
483+
!KernelBundleImpPtr->tryGetKernel(toKernelNameStrT(MKernelName),
484484
KernelBundleImpPtr)) {
485485
auto Dev =
486486
impl->MGraph ? impl->MGraph->getDevice() : MQueue->get_device();
487487
kernel_id KernelID =
488488
detail::ProgramManager::getInstance().getSYCLKernelID(
489-
MKernelName.data());
489+
toKernelNameStrT(MKernelName));
490490
bool KernelInserted = KernelBundleImpPtr->add_kernel(KernelID, Dev);
491491
// If kernel was not inserted and the bundle is in input mode we try
492492
// building it and trying to find the kernel in executable mode
@@ -550,7 +550,7 @@ event handler::finalize() {
550550
bool KernelUsesAssert =
551551
!(MKernel && MKernel->isInterop()) &&
552552
detail::ProgramManager::getInstance().kernelUsesAssert(
553-
MKernelName.data(), impl->MKernelNameBasedCachePtr);
553+
toKernelNameStrT(MKernelName), impl->MKernelNameBasedCachePtr);
554554
DiscardEvent = !KernelUsesAssert;
555555
}
556556

@@ -581,14 +581,15 @@ event handler::finalize() {
581581
#endif
582582
const detail::RTDeviceBinaryImage *BinImage = nullptr;
583583
if (detail::SYCLConfig<detail::SYCL_JIT_AMDGCN_PTX_KERNELS>::get()) {
584-
std::tie(BinImage, std::ignore) =
585-
detail::retrieveKernelBinary(*MQueue, MKernelName.data());
584+
std::tie(BinImage, std::ignore) = detail::retrieveKernelBinary(
585+
*MQueue, toKernelNameStrT(MKernelName));
586586
assert(BinImage && "Failed to obtain a binary image.");
587587
}
588588
enqueueImpKernel(
589589
MQueue, impl->MNDRDesc, impl->MArgs, KernelBundleImpPtr,
590-
MKernel.get(), MKernelName.data(), impl->MKernelNameBasedCachePtr,
591-
RawEvents, DiscardEvent ? nullptr : LastEventImpl.get(), nullptr,
590+
MKernel.get(), toKernelNameStrT(MKernelName),
591+
impl->MKernelNameBasedCachePtr, RawEvents,
592+
DiscardEvent ? nullptr : LastEventImpl.get(), nullptr,
592593
impl->MKernelCacheConfig, impl->MKernelIsCooperative,
593594
impl->MKernelUsesClusterLaunch, impl->MKernelWorkGroupMemorySize,
594595
BinImage, impl->MKernelFuncPtr, impl->MKernelNumArgs,
@@ -638,13 +639,15 @@ event handler::finalize() {
638639
std::unique_ptr<detail::CG> CommandGroup;
639640
switch (type) {
640641
case detail::CGType::Kernel: {
642+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
641643
// Copy kernel name here instead of move so that it's available after
642644
// running of this method by reductions implementation. This allows for
643645
// assert feature to check if kernel uses assertions
646+
#endif
644647
CommandGroup.reset(new detail::CGExecKernel(
645648
std::move(impl->MNDRDesc), std::move(MHostKernel), std::move(MKernel),
646649
std::move(impl->MKernelBundle), std::move(impl->CGData),
647-
std::move(impl->MArgs), MKernelName.data(),
650+
std::move(impl->MArgs), toKernelNameStrT(MKernelName),
648651
impl->MKernelNameBasedCachePtr, std::move(MStreamStorage),
649652
std::move(impl->MAuxiliaryResources), getType(),
650653
impl->MKernelCacheConfig, impl->MKernelIsCooperative,
@@ -2093,7 +2096,7 @@ backend handler::getDeviceBackend() const {
20932096

20942097
void handler::ext_intel_read_host_pipe(detail::string_view Name, void *Ptr,
20952098
size_t Size, bool Block) {
2096-
impl->HostPipeName = Name.data();
2099+
impl->HostPipeName = std::string_view(Name);
20972100
impl->HostPipePtr = Ptr;
20982101
impl->HostPipeTypeSize = Size;
20992102
impl->HostPipeBlocking = Block;
@@ -2103,7 +2106,7 @@ void handler::ext_intel_read_host_pipe(detail::string_view Name, void *Ptr,
21032106

21042107
void handler::ext_intel_write_host_pipe(detail::string_view Name, void *Ptr,
21052108
size_t Size, bool Block) {
2106-
impl->HostPipeName = Name.data();
2109+
impl->HostPipeName = std::string_view(Name);
21072110
impl->HostPipePtr = Ptr;
21082111
impl->HostPipeTypeSize = Size;
21092112
impl->HostPipeBlocking = Block;

0 commit comments

Comments
 (0)