-
Notifications
You must be signed in to change notification settings - Fork 768
[SYCL] windows shutdown fix #17124
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
[SYCL] windows shutdown fix #17124
Conversation
…design/GlobalObjectsInRuntime.md for a full technical description
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.
LGTM
if (GetModuleFileNameA(hModule, dllPath, MAX_PATH)) { | ||
char exePath[MAX_PATH]; | ||
if (GetModuleFileNameA(NULL, exePath, MAX_PATH)) { | ||
if (std::string(dllPath) == std::string(exePath)) { |
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.
std::string constructor copies char arrays, strcmp would be better.
// If loader is already closed, it'll return a not-initialized status | ||
// which the UR should convert to SUCCESS code. But that isn't always | ||
// working on Windows. This is a temporary workaround until that is fixed. | ||
// TODO: Remove this workaround when UR is fixed, and restore |
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.
could we please create a JIRA task for this todo (blocked by UR issue)?
we have a lot of TODOs in code not tracked and forgotten.
Fix for various problems in Windows shutdown. See the sycl/doc/design/GlobalObjectsInRuntime.md for the technical discussion. This was broken out from #16618 to be its own pull request.