-
Couldn't load subscription status.
- Fork 5.2k
[wasm][coreclr] Fix GetOffsetOfArgumentRegisters #120665
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
[wasm][coreclr] Fix GetOffsetOfArgumentRegisters #120665
Conversation
Fixes remaining issue mentioned in dotnet#120613 This fixes offset calculation of This and other special cased arguments. Without that fix, we ended writing the this argument value to a wrong offset.
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.
Pull Request Overview
This PR fixes offset calculation of special arguments (this pointer, return buffer, etc.) in the WASM target by introducing TARGET_REGISTER_SIZE to handle the difference between WASM's stack slot size and pointer size. On WASM, registers don't exist so arguments that would be in registers on other architectures are placed on the stack after the transition block, requiring different size calculations.
- Introduces
TARGET_REGISTER_SIZEmacro that usesINTERP_STACK_SLOT_SIZEfor WASM andTARGET_POINTER_SIZEfor other targets - Updates
GetOffsetOfArgumentRegisters()to handle WASM like AMD64 Windows ABI by placing arguments after the transition block - Replaces all
TARGET_POINTER_SIZEusage withTARGET_REGISTER_SIZEin offset calculations for special arguments
|
Tagging subscribers to this area: @mangod9 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I wonder whether it would be cleaner to replace the parts (of the arg iterator and transition block) mentioning registers on wasm. |
I think it would be cleaner to decouple the arg iterator and wasm calling convention from the interpreter implementation details. I understand that this coupling is path of the least resistance at the moment, but I am not sure whether it is going to work well once we have AOT-compiled code in the picture. |
Fixes remaining issue mentioned in #120613
This fixes offset calculation of This and other special cased arguments.
Without that fix, we ended writing the this argument value to a wrong address.