Wasmtime GitHub notifications bot (Jun 28 2023 at 18:30): Wasmtime GitHub notifications bot (Jun 28 2023 at 18:30):saulecabrera opened PR #6664 from saulecabrera:remove-assert-in-call to bytecodealliance:main:
This commits removes an assert that checked that the stack pointer position at the end of a call should be less than or equal than the position registered at the callsite.
Even though this is true in most cases, there are cases in which this is invariant is not met and as well as there are cases in which the stack pointer will inevitably be greater than the position registered at callsite:
When the call setup doesn't spill any values and instead it only consumes memory values from the value stack, the stack pointer can end up being less than what it was at the callsite.
When the call setup spills values that are not going to be consumed by the call (not used as params to the function) the stack pointer position can end up being greater than what it was at the callsite.
The assert was originally introduced to ensure the right deallocation of stack space consumed by the call, and it could be improved by applying the heuristics mentioned above, but I prefer to remove it since we already assert when emitting the epilogue that both the value stack and machine stack are in the correct state when fishing compilation.
This change includes an extra test in which the original invariant doesn't hold (case 2 described above occurs).
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Jun 28 2023 at 18:30):saulecabrera requested fitzgen for a review on PR #6664.
Wasmtime GitHub notifications bot (Jun 28 2023 at 18:30):saulecabrera requested wasmtime-compiler-reviewers for a review on PR #6664.
Wasmtime GitHub notifications bot (Jun 28 2023 at 18:30):saulecabrera requested cfallin for a review on PR #6664.
Wasmtime GitHub notifications bot (Jun 28 2023 at 18:47):cfallin submitted PR review:
Seems reasonable to me -- the coverage provided by checking at the end of the function seems adequate, as you say.
Wasmtime GitHub notifications bot (Jun 28 2023 at 18:47):cfallin has enabled auto merge for PR #6664.
Wasmtime GitHub notifications bot (Jun 28 2023 at 19:40):cfallin merged PR #6664.
Last updated: Nov 22 2024 at 16:03 UTC