-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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 passing of jit_opt_level #55897
Fix passing of jit_opt_level #55897
Conversation
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 3a71e8b (more details on the Dr. CI page): Commit 3a71e8b was recently pushed. Waiting for builds... This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
The jit_opt_level must be passed as void* directly not by reference. This avoids failures and miscompilations as the level will be kind of random instead of one of the valid values. Fixes pytorch#52147
@@ -322,7 +322,7 @@ NvrtcFunction nvrtcCompile( | |||
if (val <= 4 && val >= 0) { | |||
jit_opt_level = static_cast<uint32_t>(val); | |||
options.push_back(CU_JIT_OPTIMIZATION_LEVEL); | |||
option_vals.emplace_back(&jit_opt_level); | |||
option_vals.emplace_back((void*)jit_opt_level); |
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.
Probably a good idea to static_cast<uintptr_t>(val)
up above to guarantee that this cast to void *
is all good.
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.
Nice catch by the way! Kinda surprising this wasn't noticed before.
The jit_opt_level must be passed as void* directly not by reference.
This avoids failures and miscompilations as the level will be kind of random instead of one of the valid values.
Fixes #52147
See the issue for more details