Wasmtime GitHub notifications bot (Apr 27 2023 at 23:20): Wasmtime GitHub notifications bot (Apr 27 2023 at 23:20):alexcrichton opened PR #6302 from alexcrichton:shrink-trampolines to bytecodealliance:main:
This commit shuffles trampolines to the end of a compiled ELF file instead of interspersed throughout. Additionally trampolines are no longer given a higher alignment requirement than is required by the ISA as is given to functions since they're not perf critical.
The savings here are quite minor, only 0.3% locally on spidermonkey.wasm.
<!--
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 (Apr 27 2023 at 23:20):alexcrichton requested fitzgen for a review on PR #6302.
Wasmtime GitHub notifications bot (Apr 27 2023 at 23:20):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #6302.
Wasmtime GitHub notifications bot (Apr 27 2023 at 23:20):alexcrichton requested wasmtime-core-reviewers for a review on PR #6302.
Wasmtime GitHub notifications bot (Apr 28 2023 at 00:10):alexcrichton updated PR #6302.
Wasmtime GitHub notifications bot (Apr 28 2023 at 00:12):jameysharp submitted PR review:
Nice, I think this looks good overall. And 0.3% ain't nothing.
I would like to ask for a
struct Alignmentwith named fields forminimumandpreferred, rather than trying to match up the tuple field order with the name of the function it comes from.
Wasmtime GitHub notifications bot (Apr 28 2023 at 00:12):jameysharp created PR review comment:
What would you say to removing this method from the
compilertrait entirely? If it's only used forappend_func, which already has analignmentparameter, maybe we should just pass the desired alignment atappend_func's callers.
Wasmtime GitHub notifications bot (Apr 28 2023 at 00:12):jameysharp submitted PR review:
Nice, I think this looks good overall. And 0.3% ain't nothing.
I would like to ask for a
struct Alignmentwith named fields forminimumandpreferred, rather than trying to match up the tuple field order with the name of the function it comes from.
Wasmtime GitHub notifications bot (Apr 28 2023 at 15:07):alexcrichton updated PR #6302.
Wasmtime GitHub notifications bot (Apr 28 2023 at 17:05):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2023 at 18:02):jameysharp submitted PR review:
Other than getting very confused in one spot, this looks good to me.
Wasmtime GitHub notifications bot (Apr 28 2023 at 18:02):jameysharp submitted PR review:
Other than getting very confused in one spot, this looks good to me.
Wasmtime GitHub notifications bot (Apr 28 2023 at 18:02):jameysharp created PR review comment:
I first read this as forcing constants to be aligned to at least the function's minimum alignment, but this
alignmentvariable is not used for the constant's alignment. It's just collecting the maximum alignment of any constant or, now, the function's minimum alignment.Is there some way to make that more clear? I think renaming the variable to
function_alignmentwould have kept me from being confused, at least.
Wasmtime GitHub notifications bot (Apr 28 2023 at 22:01):alexcrichton updated PR #6302.
Wasmtime GitHub notifications bot (Apr 28 2023 at 22:01):alexcrichton created PR review comment:
Ah good point!
Wasmtime GitHub notifications bot (Apr 28 2023 at 22:01):alexcrichton has enabled auto merge for PR #6302.
Wasmtime GitHub notifications bot (Apr 28 2023 at 23:18):alexcrichton merged PR #6302.
Last updated: Dec 23 2024 at 12:05 UTC