Skip to content

Commit bbc0cf5

Browse files
ldrummldrumm
authored
[SYCL] remove alignedAllocHost's Kind param (#14862)
`alignedAllocHost` was introduced as part of what was described as a bugfix in 01869a0 wherin `alignedAlloc` was simply renamed to `alignedAllocHost`, but the `Kind` parameter (host|device|shared) was left unchanged, and the bugfix actually appears to be in the calling code. I'm not quite sure what happened there, but in any case we shouldn't have a `Kind` parameter if we only ever intend to work on the host (indeed the existing code treats Kind == host as an invariant, returning an error otherwise). It looks like a couple of other people tried to work around this when adding other features (e.g. in 7df3923) but didn't fix this due to perceived ABI breaks. However, this function hasn't been exposed as part of the abi due to a declaration / definition mismatch between the header and implementation, so it was never callable in the first place. Thus, this is a putatively NFC cleanup, but it's a change to the SYCL runtime ABI so not "genuine-NFC".
1 parent 4140240 commit bbc0cf5

File tree

2 files changed

+42
-69
lines changed

2 files changed

+42
-69
lines changed

‎sycl/include/sycl/detail/usm_impl.hpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ __SYCL_EXPORT void *alignedAlloc(size_t Alignment, size_t Bytes,
2222
sycl::usm::alloc Kind,
2323
const code_location &CL);
2424

25-
__SYCL_EXPORT void *alignedAllocHost(size_t Alignment, size_t Bytes,
26-
const context &Ctxt, sycl::usm::alloc Kind,
27-
const code_location &CL);
28-
2925
__SYCL_EXPORT void free(void *Ptr, const context &Ctxt,
3026
const code_location &CL);
3127

‎sycl/source/detail/usm/usm_impl.cpp‎

Lines changed: 42 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,16 @@
2929
#include <detail/xpti_registry.hpp>
3030
#endif
3131

32-
namespace sycl {
33-
inline namespace _V1 {
34-
35-
using alloc = sycl::usm::alloc;
36-
37-
namespace detail {
38-
#ifdef XPTI_ENABLE_INSTRUMENTATION
39-
extern xpti::trace_event_data_t *GSYCLGraphEvent;
40-
#endif
41-
namespace usm {
42-
43-
void *alignedAllocHost(size_t Alignment, size_t Size, const context &Ctxt,
44-
alloc Kind, const property_list &PropList,
45-
const detail::code_location &CodeLoc) {
32+
namespace {
33+
void *alignedAllocHost(size_t Alignment, size_t Size, const sycl::context &Ctxt,
34+
const sycl::property_list &PropList,
35+
const sycl::detail::code_location &CodeLoc) {
4636
#ifdef XPTI_ENABLE_INSTRUMENTATION
4737
// Stash the code location information and propagate
48-
detail::tls_code_loc_t CL(CodeLoc);
49-
XPTIScope PrepareNotify((void *)alignedAllocHost,
50-
(uint16_t)xpti::trace_point_type_t::node_create,
51-
SYCL_MEM_ALLOC_STREAM_NAME, "malloc_host");
38+
sycl::detail::tls_code_loc_t CL(CodeLoc);
39+
sycl::detail::XPTIScope PrepareNotify(
40+
(void *)alignedAllocHost, (uint16_t)xpti::trace_point_type_t::node_create,
41+
sycl::detail::SYCL_MEM_ALLOC_STREAM_NAME, "malloc_host");
5242
PrepareNotify.addMetadata([&](auto TEvent) {
5343
xpti::addMetadata(TEvent, "sycl_device_name", std::string("Host"));
5444
xpti::addMetadata(TEvent, "sycl_device", 0);
@@ -72,13 +62,12 @@ void *alignedAllocHost(size_t Alignment, size_t Size, const context &Ctxt,
7262
if (Size == 0)
7363
return nullptr;
7464

75-
std::shared_ptr<context_impl> CtxImpl = detail::getSyclObjImpl(Ctxt);
65+
std::shared_ptr<sycl::detail::context_impl> CtxImpl =
66+
sycl::detail::getSyclObjImpl(Ctxt);
7667
ur_context_handle_t C = CtxImpl->getHandleRef();
77-
const PluginPtr &Plugin = CtxImpl->getPlugin();
68+
const sycl::detail::PluginPtr &Plugin = CtxImpl->getPlugin();
7869
ur_result_t Error = UR_RESULT_ERROR_INVALID_VALUE;
7970

80-
switch (Kind) {
81-
case alloc::host: {
8271
ur_usm_desc_t UsmDesc{};
8372
UsmDesc.align = Alignment;
8473

@@ -100,17 +89,6 @@ void *alignedAllocHost(size_t Alignment, size_t Size, const context &Ctxt,
10089
Error = Plugin->call_nocheck(urUSMHostAlloc, C, &UsmDesc,
10190
/* pool= */ nullptr, Size, &RetVal);
10291

103-
break;
104-
}
105-
case alloc::device:
106-
case alloc::shared:
107-
case alloc::unknown: {
108-
RetVal = nullptr;
109-
Error = UR_RESULT_ERROR_INVALID_VALUE;
110-
break;
111-
}
112-
}
113-
11492
// Error is for debugging purposes.
11593
// The spec wants a nullptr returned, not an exception.
11694
if (Error != UR_RESULT_SUCCESS)
@@ -121,6 +99,18 @@ void *alignedAllocHost(size_t Alignment, size_t Size, const context &Ctxt,
12199
#endif
122100
return RetVal;
123101
}
102+
} // namespace
103+
104+
namespace sycl {
105+
inline namespace _V1 {
106+
107+
using alloc = sycl::usm::alloc;
108+
109+
namespace detail {
110+
#ifdef XPTI_ENABLE_INSTRUMENTATION
111+
extern xpti::trace_event_data_t *GSYCLGraphEvent;
112+
#endif
113+
namespace usm {
124114

125115
void *alignedAllocInternal(size_t Alignment, size_t Size,
126116
const context_impl *CtxImpl,
@@ -351,27 +341,23 @@ void free(void *ptr, const queue &Q, const detail::code_location &CodeLoc) {
351341

352342
void *malloc_host(size_t Size, const context &Ctxt,
353343
const detail::code_location &CodeLoc) {
354-
return detail::usm::alignedAllocHost(0, Size, Ctxt, alloc::host,
355-
property_list{}, CodeLoc);
344+
return alignedAllocHost(0, Size, Ctxt, property_list{}, CodeLoc);
356345
}
357346

358347
void *malloc_host(size_t Size, const context &Ctxt,
359348
const property_list &PropList,
360349
const detail::code_location &CodeLoc) {
361-
return detail::usm::alignedAllocHost(0, Size, Ctxt, alloc::host, PropList,
362-
CodeLoc);
350+
return alignedAllocHost(0, Size, Ctxt, PropList, CodeLoc);
363351
}
364352

365353
void *malloc_host(size_t Size, const queue &Q,
366354
const detail::code_location &CodeLoc) {
367-
return detail::usm::alignedAllocHost(0, Size, Q.get_context(), alloc::host,
368-
property_list{}, CodeLoc);
355+
return alignedAllocHost(0, Size, Q.get_context(), property_list{}, CodeLoc);
369356
}
370357

371358
void *malloc_host(size_t Size, const queue &Q, const property_list &PropList,
372359
const detail::code_location &CodeLoc) {
373-
return detail::usm::alignedAllocHost(0, Size, Q.get_context(), alloc::host,
374-
PropList, CodeLoc);
360+
return alignedAllocHost(0, Size, Q.get_context(), PropList, CodeLoc);
375361
}
376362

377363
void *malloc_shared(size_t Size, const device &Dev, const context &Ctxt,
@@ -401,28 +387,25 @@ void *malloc_shared(size_t Size, const queue &Q, const property_list &PropList,
401387

402388
void *aligned_alloc_host(size_t Alignment, size_t Size, const context &Ctxt,
403389
const detail::code_location &CodeLoc) {
404-
return detail::usm::alignedAllocHost(Alignment, Size, Ctxt, alloc::host,
405-
property_list{}, CodeLoc);
390+
return alignedAllocHost(Alignment, Size, Ctxt, property_list{}, CodeLoc);
406391
}
407392

408393
void *aligned_alloc_host(size_t Alignment, size_t Size, const context &Ctxt,
409394
const property_list &PropList,
410395
const detail::code_location &CodeLoc) {
411-
return detail::usm::alignedAllocHost(Alignment, Size, Ctxt, alloc::host,
412-
PropList, CodeLoc);
396+
return alignedAllocHost(Alignment, Size, Ctxt, PropList, CodeLoc);
413397
}
414398

415399
void *aligned_alloc_host(size_t Alignment, size_t Size, const queue &Q,
416400
const detail::code_location &CodeLoc) {
417-
return detail::usm::alignedAllocHost(Alignment, Size, Q.get_context(),
418-
alloc::host, property_list{}, CodeLoc);
401+
return alignedAllocHost(Alignment, Size, Q.get_context(), property_list{},
402+
CodeLoc);
419403
}
420404

421405
void *aligned_alloc_host(size_t Alignment, size_t Size, const queue &Q,
422406
const property_list &PropList,
423407
const detail::code_location &CodeLoc) {
424-
return detail::usm::alignedAllocHost(Alignment, Size, Q.get_context(),
425-
alloc::host, PropList, CodeLoc);
408+
return alignedAllocHost(Alignment, Size, Q.get_context(), PropList, CodeLoc);
426409
}
427410

428411
void *aligned_alloc_shared(size_t Alignment, size_t Size, const device &Dev,
@@ -460,25 +443,22 @@ void *malloc(size_t Size, const device &Dev, const context &Ctxt, alloc Kind,
460443
const property_list &PropList,
461444
const detail::code_location &CodeLoc) {
462445
if (Kind == alloc::host)
463-
return detail::usm::alignedAllocHost(0, Size, Ctxt, Kind, PropList,
464-
CodeLoc);
446+
return alignedAllocHost(0, Size, Ctxt, PropList, CodeLoc);
465447
return detail::usm::alignedAlloc(0, Size, Ctxt, Dev, Kind, PropList, CodeLoc);
466448
}
467449

468450
void *malloc(size_t Size, const device &Dev, const context &Ctxt, alloc Kind,
469451
const detail::code_location &CodeLoc) {
470452
if (Kind == alloc::host)
471-
return detail::usm::alignedAllocHost(0, Size, Ctxt, Kind, property_list{},
472-
CodeLoc);
453+
return alignedAllocHost(0, Size, Ctxt, property_list{}, CodeLoc);
473454
return detail::usm::alignedAlloc(0, Size, Ctxt, Dev, Kind, property_list{},
474455
CodeLoc);
475456
}
476457

477458
void *malloc(size_t Size, const queue &Q, alloc Kind,
478459
const detail::code_location &CodeLoc) {
479460
if (Kind == alloc::host)
480-
return detail::usm::alignedAllocHost(0, Size, Q.get_context(), Kind,
481-
property_list{}, CodeLoc);
461+
return alignedAllocHost(0, Size, Q.get_context(), property_list{}, CodeLoc);
482462
return detail::usm::alignedAlloc(0, Size, Q.get_context(), Q.get_device(),
483463
Kind, property_list{}, CodeLoc);
484464
}
@@ -487,8 +467,7 @@ void *malloc(size_t Size, const queue &Q, alloc Kind,
487467
const property_list &PropList,
488468
const detail::code_location &CodeLoc) {
489469
if (Kind == alloc::host)
490-
return detail::usm::alignedAllocHost(0, Size, Q.get_context(), Kind,
491-
PropList, CodeLoc);
470+
return alignedAllocHost(0, Size, Q.get_context(), PropList, CodeLoc);
492471
return detail::usm::alignedAlloc(0, Size, Q.get_context(), Q.get_device(),
493472
Kind, PropList, CodeLoc);
494473
}
@@ -497,8 +476,7 @@ void *aligned_alloc(size_t Alignment, size_t Size, const device &Dev,
497476
const context &Ctxt, alloc Kind,
498477
const detail::code_location &CodeLoc) {
499478
if (Kind == alloc::host)
500-
return detail::usm::alignedAllocHost(Alignment, Size, Ctxt, Kind,
501-
property_list{}, CodeLoc);
479+
return alignedAllocHost(Alignment, Size, Ctxt, property_list{}, CodeLoc);
502480

503481
return detail::usm::alignedAlloc(Alignment, Size, Ctxt, Dev, Kind,
504482
property_list{}, CodeLoc);
@@ -509,17 +487,16 @@ void *aligned_alloc(size_t Alignment, size_t Size, const device &Dev,
509487
const property_list &PropList,
510488
const detail::code_location &CodeLoc) {
511489
if (Kind == alloc::host)
512-
return detail::usm::alignedAllocHost(Alignment, Size, Ctxt, Kind, PropList,
513-
CodeLoc);
490+
return alignedAllocHost(Alignment, Size, Ctxt, PropList, CodeLoc);
514491
return detail::usm::alignedAlloc(Alignment, Size, Ctxt, Dev, Kind, PropList,
515492
CodeLoc);
516493
}
517494

518495
void *aligned_alloc(size_t Alignment, size_t Size, const queue &Q, alloc Kind,
519496
const detail::code_location &CodeLoc) {
520497
if (Kind == alloc::host)
521-
return detail::usm::alignedAllocHost(Alignment, Size, Q.get_context(), Kind,
522-
property_list{}, CodeLoc);
498+
return alignedAllocHost(Alignment, Size, Q.get_context(), property_list{},
499+
CodeLoc);
523500
return detail::usm::alignedAlloc(Alignment, Size, Q.get_context(),
524501
Q.get_device(), Kind, property_list{},
525502
CodeLoc);
@@ -529,8 +506,8 @@ void *aligned_alloc(size_t Alignment, size_t Size, const queue &Q, alloc Kind,
529506
const property_list &PropList,
530507
const detail::code_location &CodeLoc) {
531508
if (Kind == alloc::host)
532-
return detail::usm::alignedAllocHost(Alignment, Size, Q.get_context(), Kind,
533-
PropList, CodeLoc);
509+
return alignedAllocHost(Alignment, Size, Q.get_context(), PropList,
510+
CodeLoc);
534511
return detail::usm::alignedAlloc(Alignment, Size, Q.get_context(),
535512
Q.get_device(), Kind, PropList, CodeLoc);
536513
}

0 commit comments

Comments
 (0)