-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 signature for 64-bit delegate profiling helper #74500
Fix signature for 64-bit delegate profiling helper #74500
Conversation
I missed updating the signature of the 64-bit variant here when the vtable and delegate profiling helpers were originally split up. Fix dotnet#74295
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsI missed updating the signature of the 64-bit variant here when the Fix #74295
|
Add some testing.
I've added a cc @dotnet/jit-contrib PTAL @EgorBo @AndyAyersMS |
/azp run runtime-coreclr pgo |
Azure Pipelines successfully started running 1 pipeline(s). |
Bunch of the runs seem to have been "dead-lettered".. will retry |
/azp run runtime-coreclr pgo |
Azure Pipelines successfully started running 1 pipeline(s). |
Would it be worth devising a way for the compiler to assert that it has the right types (or at least number) of arguments for helper calls? Presumably these issues are going to be caught by normal testing, so it would be more a productivity thing, so is this kind of issue common? |
This needs some work so disable it for now.
I'm not sure if the effort would be worth it, I don't think I've seen this kind of mismatch before. But we should definitely ensure that we have testing to exercise all the helpers and the code in the JIT that builds the calls to them. |
/azp run runtime-coreclr pgo |
Azure Pipelines successfully started running 1 pipeline(s). |
@jakobbotsch are we going to backport this to 7.0? |
Yes, I will backport this one. |
/backport to release/7.0 |
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2947516086 |
I missed updating the signature of the 64-bit variant here when the
vtable and delegate profiling helpers were originally split up.
Fix #74295