-
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 assert for ARM shuffle thunk #66642
Conversation
Handle case where gap exists at the beginning of the stack range. Fix dotnet#13241
This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did. For the test case at #13241 (comment), the generated thunk is:
with the "// gap" lines being generated when the assertion was previously hitting. Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not... |
@janvorli Who knows about (ARM) shuffle thunks? cc @jakobbotsch |
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, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.
@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553 |
Handle case where gap exists at the beginning of the stack range. Fix dotnet#13241
Handle case where gap exists at the beginning of the stack
range.
Fix #13241