-
Notifications
You must be signed in to change notification settings - Fork 3k
Eliminate the set_tuple_element instruction #10144
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
Eliminate the set_tuple_element instruction #10144
Conversation
CT Test Results 4 files 457 suites 54m 23s ⏱️ Results for commit a144913. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
Do the sequences of |
No. Optimization of |
@@ -1102,6 +1102,11 @@ void BeamModuleAssembler::emit_update_record_in_place( | |||
void BeamModuleAssembler::emit_set_tuple_element(const ArgSource &Element, | |||
const ArgRegister &Tuple, | |||
const ArgWord &Offset) { | |||
/* TODO: As of Erlang/OTP 29, this instruction is no longer | |||
* emitted by the compiler. It can be removed when the runtime | |||
* system no longer supports loading code compiled by Erlang/OTP |
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.
Thank you for this!
@@ -204,8 +204,6 @@ validate_0([{function, Name, Arity, Entry, Code} | Fs], Module, Level, Ft) -> | |||
hf=0, | |||
%% List of hot catch/try tags | |||
ct=[], | |||
%% Previous instruction was setelement/3. | |||
setelem=false, |
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.
We can do a drive-by removal of puts_left
too, it's not used anywhere.
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.
Good idea. Done.
The `set_tuple_element` instruction was used before OTP 26 to optimize chains of `setelement/3` calls. For example: foo(S0) when tuple_size(S0) =:= 8 -> S1 = setelement(8, S0, a), S2 = setelement(7, S1, b), setelement(5, S2, c). The compiler would keep the first call to `setelement/3` and replace the other two with `set_tuple_element`, which would destructively update the tuple created by the `setelement/3` call. Starting in OTP 26, this was changed so that all three `setelement/3` calls are replaced with a single `update_record` instruction. The `set_tuple_element` instruction was only used when the size of the tuple being updated was not known at compile time, as in the following example: bar(S0) -> S1 = setelement(8, S0, a), S2 = setelement(7, S1, b), setelement(5, S2, c). The only difference compared to the previous example is that the type of `S0` is not known. When the size of the tuple cannot be determined at compile time, the compiler cannot use `update_record` and must fall back to `setelement/3` and `set_tuple_element`. However, the way this fallback was implemented caused extra useless instructions to be generated (see erlang#10125 for examples). This commit eliminates the use of `set_tuple_element` in the fallback path, allowing us to remove some particularly messy code from `beam_validator`. Resolves erlang#10125
596432e
to
a144913
Compare
The
set_tuple_element
instruction was used before OTP 26 to optimize chains ofsetelement/3
calls. For example:The compiler would keep the first call to
setelement/3
and replace the other two withset_tuple_element
, which would destructively update the tuple created by thesetelement/3
call.Starting in OTP 26, this was changed so that all three
setelement/3
calls are replaced with a singleupdate_record
instruction.The
set_tuple_element
instruction was only used when the size of the tuple being updated was not known at compile time, as in the following example:The only difference compared to the previous example is that the type of
S0
is not known. When the size of the tuple cannot be determined at compile time, the compiler cannot useupdate_record
and must fall back tosetelement/3
andset_tuple_element
. However, the way this fallback was implemented caused extra useless instructions to be generated (see #10125 for examples).This commit eliminates the use of
set_tuple_element
in the fallback path, allowing us to remove some particularly messy code frombeam_validator
.Resolves #10125