Skip to content

Conversation

@ltratt
Copy link
Contributor

@ltratt ltratt commented Apr 27, 2025

Before this commit, we loaded a closure's proto_version on every iteration of the loop, which leads to a guard in every opcode in a trace.

This commit hoists that load out to the only point where that value can change -- the startfunc/return label(s). These are where a new closure is dealt with (e.g. as a result of OP_CALL). Although we still have to call yk_promote for every opcode, the trace optimiser has a much easier job, because this tends to stay in a variable -- in general it can now remove 1 guard for all but the first opcode. Across several benchmarks I've looked at, this leads to a 2-6% increase.

Before this commit, we loaded a closure's `proto_version` on every
iteration of the loop, which leads to a guard in every opcode in a
trace.

This commit hoists that load out to the only point where that value can
change -- the `startfunc`/`return` label(s). These are where a new
closure is dealt with (e.g. as a result of `OP_CALL`). Although we still
have to call `yk_promote` for every opcode, the trace optimiser has a
much easier job, because this tends to stay in a variable -- in general
it can now remove 1 guard for all but the first opcode. Across several
benchmarks I've looked at, this leads to a 2-6% increase.
@vext01 vext01 added this pull request to the merge queue Apr 28, 2025
@vext01 vext01 removed this pull request from the merge queue due to a manual request Apr 28, 2025
@vext01
Copy link
Contributor

vext01 commented Apr 28, 2025

LGTM

@ptersilie ptersilie added this pull request to the merge queue Apr 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 28, 2025
@vext01 vext01 added this pull request to the merge queue Apr 28, 2025
Merged via the queue into ykjit:main with commit a72bcc2 Apr 28, 2025
2 checks passed
@ltratt ltratt deleted the hoist_load branch May 30, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants