-
Notifications
You must be signed in to change notification settings - Fork 44
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
Make sure ext_oneapi_get_default_context
doesn't broke runtime on windows
#2742
Conversation
…on windows Signed-off-by: Anatoly Myachev <[email protected]>
2e1eeed
to
03a32c5
Compare
third_party/intel/backend/driver.c
Outdated
@@ -194,8 +194,18 @@ static PyObject *loadBinary(PyObject *self, PyObject *args) { | |||
const size_t binary_size = PyBytes_Size(py_bytes); | |||
|
|||
uint8_t *binary_ptr = (uint8_t *)PyBytes_AsString(py_bytes); | |||
const auto ctx = | |||
sycl_device.get_platform().ext_oneapi_get_default_context(); | |||
auto platform = sycl_device.get_platform(); |
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.
We already have exceptions in this code block - but I suppose I might feel better if this was in a free function that had compile time definitions for windows (where we need the exception handling) and linux (where we don't).
Also, how can the code properly run with no valid context? What if a valid context is thrown on linux?
(I realize I probably approved the Windows PR that has this change but missed the details over there - maybe we should split some other changes out of the Windows PR for a more thorough individual review).
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.
Done
Also, how can the code properly run with no valid context?
This is a explanation for Windows case: https://github.com/intel/intel-xpu-backend-for-triton/pull/2478/files#r1844125901
Signed-off-by: Anatoly Myachev <[email protected]>
92879d1
to
d132f22
Compare
I found how pytorch implements the same functionality on Linux and Windows. You may want to take a look. |
This is interesting, but it will also require us to implement |
Signed-off-by: Anatoly Myachev <[email protected]>
Signed-off-by: Anatoly Myachev <[email protected]>
@alexbaden ready for review |
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.
looks fine for now
Part of #2478 (to reduce diff)
These are quite stable changes, we can merge it without CI on Windows. @gshimansky if you don't mind.