-
Notifications
You must be signed in to change notification settings - Fork 732
(refactor): Build jump and jump table to support prepended instructions. #9463
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
(refactor): Build jump and jump table to support prepended instructions. #9463
Conversation
orizi
left a comment
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.
@orizi made 4 comments.
Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @eytan-starkware, @ilyalesokhin-starkware, and @TomerStarkware).
crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 234 at r1 (raw file):
>, ) -> Result<CompiledInvocation, InvocationError> { build_enum_match_short_ex(builder, variant_selector, output_expressions, vec![])
Suggestion:
build_enum_match_short_ex(builder, variant_selector, output_expressions, Vec::with_capacity(1))crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 247 at r1 (raw file):
) -> Result<CompiledInvocation, InvocationError> { let mut relocations = Vec::new(); let relocation_offset = instructions.len();
Suggestion:
let base_instruction_count = instructions.len();crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 301 at r1 (raw file):
>, ) -> Result<CompiledInvocation, InvocationError> { build_enum_match_long_ex(builder, variant_selector, output_expressions, vec![])
Suggestion:
build_enum_match_long_ex(builder, variant_selector, output_expressions, , Vec::with_capacity(builder.invocation.branches.len() + 1))crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 318 at r1 (raw file):
}); let relocation_offset = instructions.len();
Suggestion:
let base_instruction_count = instructions.len();254407f to
7de9b91
Compare
f4ea7ec to
36c875a
Compare
eytan-starkware
left a comment
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.
@eytan-starkware made 4 comments.
Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware, @orizi, and @TomerStarkware).
crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 234 at r1 (raw file):
>, ) -> Result<CompiledInvocation, InvocationError> { build_enum_match_short_ex(builder, variant_selector, output_expressions, vec![])
Done.
crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 247 at r1 (raw file):
) -> Result<CompiledInvocation, InvocationError> { let mut relocations = Vec::new(); let relocation_offset = instructions.len();
Done.
crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 301 at r1 (raw file):
>, ) -> Result<CompiledInvocation, InvocationError> { build_enum_match_long_ex(builder, variant_selector, output_expressions, vec![])
Done.
crates/cairo-lang-sierra-to-casm/src/invocations/enm.rs line 318 at r1 (raw file):
}); let relocation_offset = instructions.len();
Done.
TomerStarkware
left a comment
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.
@TomerStarkware partially reviewed 1 file and made 1 comment.
Reviewable status: all files reviewed (commit messages unreviewed), 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @orizi).
36c875a to
612b112
Compare
7de9b91 to
2f62226
Compare
2f62226 to
68b0a7d
Compare
612b112 to
a0f42e0
Compare
68b0a7d to
c4c9d0f
Compare
a0f42e0 to
4e956b9
Compare
orizi
left a comment
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.
@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 4 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware).
4e956b9 to
7b6d2af
Compare
c4c9d0f to
7638bf8
Compare
7b6d2af to
a793aa3
Compare
7638bf8 to
902d2d9
Compare
a793aa3 to
2ba5194
Compare
09cc3cb to
dc02369
Compare
2ba5194 to
0e998a4
Compare
dc02369 to
900b6c4
Compare
0e998a4 to
442b9c0
Compare
442b9c0 to
de22370
Compare
900b6c4 to
b5e7058
Compare
SIERRA_UPDATE_MINOR_CHANGE_TAG=Refactored jump table building functions
b5e7058 to
4503ac9
Compare

Summary
Added extended versions of
build_enum_match_shortandbuild_enum_match_longfunctions that allow prepending instructions. The new functionsbuild_enum_match_short_exandbuild_enum_match_long_extake an additional parameter for initial instructions and properly handle relocation offsets. The original functions were refactored to call these extended versions with empty instruction vectors.Type of change
Please check one:
Why is this change needed?
This change enables more flexibility when generating code for enum matching operations by allowing custom instructions to be prepended before the standard matching logic. This is particularly useful for complex enum operations that require setup code before the actual matching logic.
What was the behavior or documentation before?
Previously, the enum matching functions could only generate a fixed set of instructions without the ability to prepend custom setup instructions.
What is the behavior or documentation after?
Now, there are extended versions of the enum matching functions that accept an additional parameter for initial instructions. The original functions remain unchanged in behavior but internally call the new extended versions.
Additional context
The implementation carefully handles relocation offsets to ensure that jump targets remain correct when instructions are prepended. This change maintains backward compatibility while providing more flexibility for code generation.