Skip to content

Commit fb8ddbf

Browse files
authored
Allow passing default constructed accessors to kernels (#10780)
Prior to this PR, we would throw or crash when a default constructed accessor was passed to a kernel if it was not filtered out, even if it wasn't really accessed. Passing a default constructed accessor to a kernel is allowed by SYCL2020, and this PR ensures our implementation allows it. --------- Signed-off-by: Maronas, Marcos <[email protected]>
1 parent 3f3df77 commit fb8ddbf

File tree

2 files changed

+81
-7
lines changed

2 files changed

+81
-7
lines changed

sycl/source/detail/scheduler/commands.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,9 +1537,10 @@ AllocaCommandBase *ExecCGCommand::getAllocaForReq(Requirement *Req) {
15371537
if (Dep.MDepRequirement == Req)
15381538
return Dep.MAllocaCmd;
15391539
}
1540-
throw sycl::exception(sycl::make_error_code(sycl::errc::runtime),
1541-
"Alloca for command not found " +
1542-
codeToString(PI_ERROR_INVALID_OPERATION));
1540+
// Default constructed accessors do not add dependencies, but they can be
1541+
// passed to commands. Simply return nullptr, since they are empty and don't
1542+
// really require any memory.
1543+
return nullptr;
15431544
}
15441545

15451546
std::vector<std::shared_ptr<const void>>
@@ -2213,11 +2214,15 @@ void SetArgBasedOnType(
22132214
break;
22142215
case kernel_param_kind_t::kind_accessor: {
22152216
Requirement *Req = (Requirement *)(Arg.MPtr);
2216-
assert(getMemAllocationFunc != nullptr &&
2217-
"We should have caught this earlier.");
22182217

2218+
// getMemAllocationFunc is nullptr when there are no requirements. However,
2219+
// we may pass default constructed accessors to a command, which don't add
2220+
// requirements. In such case, getMemAllocationFunc is nullptr, but it's a
2221+
// valid case, so we need to properly handle it.
22192222
sycl::detail::pi::PiMem MemArg =
2220-
(sycl::detail::pi::PiMem)getMemAllocationFunc(Req);
2223+
getMemAllocationFunc
2224+
? (sycl::detail::pi::PiMem)getMemAllocationFunc(Req)
2225+
: nullptr;
22212226
if (Context.get_backend() == backend::opencl) {
22222227
// clSetKernelArg (corresponding to piKernelSetArg) returns an error
22232228
// when MemArg is null, which is the case when zero-sized buffers are
@@ -2831,7 +2836,9 @@ pi_int32 ExecCGCommand::enqueueImpQueue() {
28312836

28322837
auto getMemAllocationFunc = [this](Requirement *Req) {
28332838
AllocaCommandBase *AllocaCmd = getAllocaForReq(Req);
2834-
return AllocaCmd->getMemAllocation();
2839+
// getAllocaForReq may return nullptr if Req is a default constructed
2840+
// accessor. Simply return nullptr in such a case.
2841+
return AllocaCmd ? AllocaCmd->getMemAllocation() : nullptr;
28352842
};
28362843

28372844
const std::shared_ptr<detail::kernel_impl> &SyclKernel =

sycl/test-e2e/Basic/accessor/accessor.cpp

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,5 +1491,72 @@ int main() {
14911491
assert(!result);
14921492
}
14931493

1494+
// default constructed accessor can be passed to a kernel.
1495+
{
1496+
AccT acc;
1497+
sycl::queue q;
1498+
bool result;
1499+
{
1500+
sycl::buffer<bool, 1> Buf{&result, sycl::range<1>{1}};
1501+
// We are passing a default constructed accessor and a non default
1502+
// constructed accessor with storage. Default constructed accessors can be
1503+
// passed to commands, but trying to access the (non-existing) underlying
1504+
// storage is UB. This test should work, since the access to the default
1505+
// constructed accessor must never be reached.
1506+
try {
1507+
q.submit([&](sycl::handler &cgh) {
1508+
sycl::accessor res_acc{Buf, cgh};
1509+
cgh.single_task<class def_ctor_kernel>([=] {
1510+
if (false)
1511+
res_acc[0] = acc[0];
1512+
});
1513+
});
1514+
q.wait_and_throw();
1515+
} catch (sycl::exception &e) {
1516+
assert("Unexpected exception");
1517+
} catch (...) {
1518+
std::cout << "Some other unexpected exception (line " << __LINE__ << ")"
1519+
<< std::endl;
1520+
return 1;
1521+
}
1522+
}
1523+
assert(!result);
1524+
}
1525+
1526+
// default constructed accessor can be passed to a kernel (2).
1527+
{
1528+
using AccT = sycl::accessor<int, 1, sycl::access::mode::read_write>;
1529+
AccT acc;
1530+
assert(acc.empty());
1531+
sycl::queue q;
1532+
bool result;
1533+
{
1534+
// We are passing only a default constructed accessor. Default constructed
1535+
// accessors can be passed to commands, but trying to access the
1536+
// (non-existing) underlying storage is UB. This test should work, since
1537+
// the access to the default constructed accessor must never be reached.
1538+
// The difference with the previous test case is that in this case the
1539+
// task will not have any requirements, while the previous one does have
1540+
// one requirement for the non default constructed accessor, testing
1541+
// different code paths.
1542+
try {
1543+
q.submit([&](sycl::handler &cgh) {
1544+
cgh.single_task<class def_ctor_kernel2>([=] {
1545+
if (!acc.empty())
1546+
acc[0] = 1;
1547+
});
1548+
});
1549+
q.wait_and_throw();
1550+
} catch (sycl::exception &e) {
1551+
assert("Unexpected exception");
1552+
} catch (...) {
1553+
std::cout << "Some other unexpected exception (line " << __LINE__ << ")"
1554+
<< std::endl;
1555+
return 1;
1556+
}
1557+
}
1558+
assert(!result);
1559+
}
1560+
14941561
std::cout << "Test passed" << std::endl;
14951562
}

0 commit comments

Comments
 (0)