khepri_fun: Resolve label inside {call_fun2, {u,Idx}, ...} instruction
#149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportBase: 90.82% // Head: 90.99% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #149 +/- ##
==========================================
+ Coverage 90.82% 90.99% +0.16%
==========================================
Files 16 16
Lines 2868 2886 +18
==========================================
+ Hits 2605 2626 +21
+ Misses 263 260 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
FTR,
The code to decode all chunks is in |
In pull request #134, the `{u,Idx}` argument was discarded and replaced by `{atom, safe}` because the module was already compiled successfully and the register used in the instruction must point to a valid function. `Idx` is an offset in the Lambda table ("FunT" in the beam chunks). In this patch, we decode this table. This allows us to resolve the label used by `call_fun2` and replace `{u,Idx}` by a valid `{f,Label}`. This is probably a small optimization of the resulting code, but it closer to what the Erlang compiler would do. The testcase is expanded to execute the extracted function to make sure it still works.
dumbbell
force-pushed
the
resolve-label-in-call_fun2
branch
from
51810f0
to
958169b
Compare
the-mikedavis
approved these changes
Oct 11, 2022
dumbbell
added a commit
that referenced
this pull request
Nov 2, 2022
Since #149, we resolve and adjust the jump label inside the `call_fun2` instruction. This way we are as close as possible to the code originally produced by the compiler and benefit from its optimizations. However, there is one situation where we can't keep the jump label: it is when the called local function is not local anymore after extraction. To avoid many almost identical copies of the same anonymous function where only the environment changes, `khepri_fun` will extract the function in a dedicated module and keep the environment separate. If there is another anonymous function inside this environment, it will become its own module. Therefore, it's possible that in the original Beam code, the two anonymous functions were inside the same module. If the compiler emitted a `call_fun2` instruction, it could use a jump label in the outer function to call the inner function passed in the environment. After extraction by `khepri_fun`, this is no longer possible because the inner function will be in another module. `call_fun2` must use the `{atom, safe}` argument instead.
dumbbell
added a commit
that referenced
this pull request
Nov 2, 2022
Since #149, we resolve and adjust the jump label inside the `call_fun2` instruction. This way we are as close as possible to the code originally produced by the compiler and benefit from its optimizations. However, there is one situation where we can't keep the jump label: it is when the called local function is not local anymore after extraction. To avoid many almost identical copies of the same anonymous function where only the environment changes, `khepri_fun` will extract the function in a dedicated module and keep the environment separate. If there is another anonymous function inside this environment, it will become its own module. Therefore, it's possible that in the original Beam code, the two anonymous functions were inside the same module. If the compiler emitted a `call_fun2` instruction, it could use a jump label in the outer function to call the inner function passed in the environment. After extraction by `khepri_fun`, this is no longer possible because the inner function will be in another module. `call_fun2` must use the `{atom, safe}` argument instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In pull request #134, the
{u,Idx}argument was discarded and replaced by{atom, safe}because the module was already compiled successfully and the register used in the instruction must point to a valid function.Idxis an offset in the Lambda table ("FunT" in the beam chunks). In this patch, we decode this table. This allows us to resolve the label used bycall_fun2and replace{u,Idx}by a valid{f,Label}.This is probably a small optimization of the resulting code, but it closer to what the Erlang compiler would do.
The testcase is expanded to execute the extracted function to make sure it still works.