Skip to content

[SYCL] Fix handling of queue::in_order property #2140

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

Merged
merged 2 commits into from
Jul 20, 2020
Merged
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
17 changes: 8 additions & 9 deletions sycl/source/detail/queue_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,14 @@ class queue_impl {
/// \param Device is a SYCL device that is used to dispatch tasks submitted
/// to the queue.
/// \param AsyncHandler is a SYCL asynchronous exception handler.
/// \param Order specifies whether the queue being constructed as in-order
/// or out-of-order.
/// \param PropList is a list of properties to use for queue construction.
queue_impl(DeviceImplPtr Device, async_handler AsyncHandler, QueueOrder Order,
queue_impl(DeviceImplPtr Device, async_handler AsyncHandler,
const property_list &PropList)
: queue_impl(Device,
detail::getSyclObjImpl(context(
createSyclObjFromImpl<device>(Device), {},
(DefaultContextType == cuda_context_type::primary))),
AsyncHandler, Order, PropList){};
AsyncHandler, PropList){};

/// Constructs a SYCL queue with an async_handler and property_list provided
/// form a device and a context.
Expand All @@ -69,12 +67,9 @@ class queue_impl {
/// \param Context is a SYCL context to associate with the queue being
/// constructed.
/// \param AsyncHandler is a SYCL asynchronous exception handler.
/// \param Order specifies whether the queue being constructed as in-order
/// or out-of-order.
/// \param PropList is a list of properties to use for queue construction.
queue_impl(DeviceImplPtr Device, ContextImplPtr Context,
async_handler AsyncHandler, QueueOrder Order,
const property_list &PropList)
async_handler AsyncHandler, const property_list &PropList)
: MDevice(Device), MContext(Context), MAsyncHandler(AsyncHandler),
MPropList(PropList), MHostQueue(MDevice->is_host()),
MOpenCLInterop(!MHostQueue) {
Expand All @@ -84,7 +79,11 @@ class queue_impl {
"as the context does not contain the given device.",
PI_INVALID_DEVICE);
if (!MHostQueue) {
MCommandQueue = createQueue(Order);
const QueueOrder qorder =
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny comment: qorder -> QOrder or QO (please start var names with a capital letter)

MPropList.has_property<property::queue::in_order>()
? QueueOrder::Ordered
: QueueOrder::OOO;
MCommandQueue = createQueue(qorder);
}
}

Expand Down
2 changes: 1 addition & 1 deletion sycl/source/detail/scheduler/scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ Scheduler::Scheduler() {
sycl::device HostDevice;
DefaultHostQueue = QueueImplPtr(
new queue_impl(detail::getSyclObjImpl(HostDevice), /*AsyncHandler=*/{},
QueueOrder::Ordered, /*PropList=*/{}));
/*PropList=*/{}));
}

void Scheduler::lockSharedTimedMutex(
Expand Down
18 changes: 3 additions & 15 deletions sycl/source/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,6 @@
__SYCL_INLINE_NAMESPACE(cl) {
namespace sycl {

namespace detail {

QueueOrder getQueueOrder(const property_list &propList) {
if (propList.has_property<property::queue::in_order>()) {
return QueueOrder::Ordered;
}
return QueueOrder::OOO;
}

} // namespace detail

queue::queue(const context &syclContext, const device_selector &deviceSelector,
const async_handler &asyncHandler, const property_list &propList) {

Expand All @@ -42,7 +31,7 @@ queue::queue(const context &syclContext, const device_selector &deviceSelector,

impl = std::make_shared<detail::queue_impl>(
detail::getSyclObjImpl(syclDevice), detail::getSyclObjImpl(syclContext),
asyncHandler, detail::getQueueOrder(propList), propList);
asyncHandler, propList);
}

queue::queue(const context &syclContext,
Expand All @@ -51,14 +40,13 @@ queue::queue(const context &syclContext,
const property_list &propList) {
impl = std::make_shared<detail::queue_impl>(
detail::getSyclObjImpl(syclDevice), detail::getSyclObjImpl(syclContext),
asyncHandler, cl::sycl::detail::QueueOrder::OOO, propList);
asyncHandler, propList);
}

queue::queue(const device &syclDevice, const async_handler &asyncHandler,
const property_list &propList) {
impl = std::make_shared<detail::queue_impl>(
detail::getSyclObjImpl(syclDevice), asyncHandler,
detail::getQueueOrder(propList), propList);
detail::getSyclObjImpl(syclDevice), asyncHandler, propList);
}

queue::queue(cl_command_queue clQueue, const context &syclContext,
Expand Down
29 changes: 25 additions & 4 deletions sycl/test/inorder_queue/prop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ bool getQueueOrder(cl_command_queue cq) {
: true;
}

int main() {
queue q{property::queue::in_order()};
int CheckQueueOrder(const queue &q) {
auto dev = q.get_device();
cl_command_queue cq = q.get();

cl_command_queue cq = q.get();
bool expected_result = dev.is_host() ? true : getQueueOrder(cq);
if (!expected_result)
return -1;
Expand All @@ -44,3 +43,25 @@ int main() {

return 0;
}

int main() {
queue q1{property::queue::in_order()};
int res = CheckQueueOrder(q1);
if (res != 0)
return res;

device dev{cl::sycl::default_selector{}};
context ctx{dev};

auto exception_handler = [](cl::sycl::exception_list exceptions) {
};

queue q2{
ctx, dev, exception_handler, {sycl::property::queue::in_order()}};

res = CheckQueueOrder(q2);
if (res != 0)
return res;

return 0;
}
2 changes: 1 addition & 1 deletion sycl/unittests/scheduler/CommandsWaitForEvents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ TEST_F(SchedulerTest, CommandsWaitForEvents) {
sycl::device HostDevice;
std::shared_ptr<detail::queue_impl> DefaultHostQueue(new detail::queue_impl(
detail::getSyclObjImpl(HostDevice), /*AsyncHandler=*/{},
detail::QueueOrder::Ordered, /*PropList=*/{}));
/*PropList=*/{}));

MockCommand Cmd(DefaultHostQueue);

Expand Down