-
Notifications
You must be signed in to change notification settings - Fork 83
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
Fix kernel_bundle tests for multi-device systems #878
Fix kernel_bundle tests for multi-device systems #878
Conversation
Some of the kernel_bundle tests make faulty assumptions about the set of devices used in the kernel bundles. This commit fixes these assumptions. Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
tests/common/common.h
Outdated
auto device_order_f = [](const sycl::device& d1, | ||
const sycl::device& d2) -> bool { | ||
std::hash<sycl::device> h{}; | ||
return h(d1) < h(d2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we added that in the spec.
But not yet: https://gitlab.khronos.org/sycl/Specification/-/issues/586
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But is this a working order?
What happens when we have 2 different devices with the same hash value?
I am afraid that while we have not solved gitlab.khronos.org/sycl/Specification/-/issues/586 we are stuck with a quadratic algorithm. :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, that's a good point. I've changed it to the simple, but valid, implementation.
Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@steffenlarsen We should make a common proposal for the next F2F on "Make SYCL types with reference semantics totally ordered" since you have enough suffered.
Some of the kernel_bundle tests make faulty assumptions about the set of devices used in the kernel bundles. This commit fixes these assumptions.