-
Notifications
You must be signed in to change notification settings - Fork 702
added specialization for recursive functions #8881
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
base: main
Are you sure you want to change the base?
added specialization for recursive functions #8881
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 reviewed 1 of 6 files at r1, all commit messages.
Reviewable status: 1 of 6 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/lower/generated_test.rs line 69 at r1 (raw file):
) .unwrap(); let calls = lowering
doc what you are doing here.
9be13c0 to
b110587
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 of 6 files at r1, 7 of 8 files at r2.
Reviewable status: 8 of 9 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 719 at r2 (raw file):
} _ => vec![None; call_stmt.inputs.len()], };
Suggestion:
let self_specializition =
if let ConcreteFunctionWithBodyLongId::Specialized(specialized) = self.caller_function &&
caller_base == called_base {
specialized.args.iter().map(Some).collect()
} else {
vec![None; call_stmt.inputs.len()]
};
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.
Reviewable status: 8 of 9 files reviewed, 2 unresolved discussions (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1330 at r2 (raw file):
ty: TypeId<'db>, unknown_vars: &mut Vec<VarUsage<'db>>, coerce: Option<&SpecializationArg<'db>>,
add doc about the coercion.
b110587 to
fc53245
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 of 8 files at r2, all commit messages.
Reviewable status: 9 of 11 files reviewed, 3 unresolved discussions (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1455 at r3 (raw file):
unreachable!("Expected an enum specialization argument"); }; assert_eq!(coercion_variant, &variant);
shouldn't this just return None?
Code quote:
assert_eq!(coercion_variant, &variant);fc53245 to
334a183
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 10 of 10 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @TomerStarkware)
334a183 to
1a346e4
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 3 of 8 files at r5, all commit messages.
Reviewable status: 6 of 11 files reviewed, 2 unresolved discussions (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1423 at r5 (raw file):
Some(coerce) => { let SpecializationArg::Struct(specialization_args) = coerce else { unreachable!("Expected a struct specialization argument");
reachable ::Const as well.
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1457 at r5 (raw file):
let SpecializationArg::Enum { variant: coercion_variant, payload } = coerce else { unreachable!("Expected an enum specialization argument");
reachable ::Const as well.
1a346e4 to
0ba4dcb
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 4 of 8 files at r5, 8 of 8 files at r6, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)
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.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/lower/generated_test.rs line 69 at r1 (raw file):
Previously, orizi wrote…
doc what you are doing here.
Done.
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1330 at r2 (raw file):
Previously, orizi wrote…
add doc about the coercion.
Done.
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1455 at r3 (raw file):
Previously, orizi wrote…
shouldn't this just return None?
Done.
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 1457 at r5 (raw file):
Previously, orizi wrote…
reachable ::Const as well.
changed in previous pr
crates/cairo-lang-lowering/src/optimizations/const_folding.rs line 719 at r2 (raw file):
} _ => vec![None; call_stmt.inputs.len()], };
Done.
0ba4dcb to
fdbbc36
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 7 of 7 files at r7, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)

TL;DR
Improve function specialization in the lowering phase by tracking specialized function calls
What changed?