Add the same optimization to dependent functions

I originally didn't add this because I thought this was mutually
recursive functions, which I couldn't picture how that would work;

I refactored all this logic into modify_self_calls, which maybe needs a
better name now.

Perf gain on some stdlib tests (line concat tests) is 93%!!
This commit is contained in:
Pi Lanningham 2023-08-03 12:52:38 -04:00 committed by Kasey
parent c45caaefc8
commit fc948f0029
2 changed files with 63 additions and 67 deletions

View File

@ -2,7 +2,7 @@ pub mod air;
pub mod builder; pub mod builder;
pub mod tree; pub mod tree;
use std::{sync::Arc, collections::HashMap}; use std::sync::Arc;
use indexmap::{IndexMap, IndexSet}; use indexmap::{IndexMap, IndexSet};
use itertools::Itertools; use itertools::Itertools;
@ -26,7 +26,7 @@ use crate::{
convert_opaque_type, erase_opaque_type_operations, find_and_replace_generics, convert_opaque_type, erase_opaque_type_operations, find_and_replace_generics,
get_arg_type_name, get_generic_id_and_type, get_variant_name, monomorphize, get_arg_type_name, get_generic_id_and_type, get_variant_name, monomorphize,
pattern_has_conditions, wrap_as_multi_validator, wrap_validator_condition, CodeGenFunction, pattern_has_conditions, wrap_as_multi_validator, wrap_validator_condition, CodeGenFunction,
SpecificClause, identify_recursive_static_params, SpecificClause,
}, },
tipo::{ tipo::{
ModuleValueConstructor, PatternConstructor, Type, TypeInfo, ValueConstructor, ModuleValueConstructor, PatternConstructor, Type, TypeInfo, ValueConstructor,
@ -2695,36 +2695,12 @@ impl<'a> CodeGenerator<'a> {
let deps = (tree_path, func_deps.clone()); let deps = (tree_path, func_deps.clone());
if !params_empty { if !params_empty {
let mut potential_recursive_statics = vec![]; let recursive_nonstatics = if is_recursive {
if is_recursive { modify_self_calls(&mut body, key, variant, &func_params)
potential_recursive_statics = func_params.clone(); } else {
// identify which parameters are recursively nonstatic (i.e. get modified before the self-call) func_params.clone()
// TODO: this would be a lot simpler if each `Var`, `Let`, function argument, etc. had a unique identifier };
// rather than just a name; this would let us track if the Var passed to itself was the same value as the method argument
let mut shadowed_parameters: HashMap<String, TreePath> = HashMap::new();
body.traverse_tree_with(&mut |air_tree: &mut AirTree, tree_path| {
identify_recursive_static_params(air_tree, tree_path, &func_params, key, variant, &mut shadowed_parameters, &mut potential_recursive_statics)
});
// Find the index of any recursively static parameters,
// so we can remove them from the call-site of each recursive call
let recursive_static_indexes = func_params
.iter()
.enumerate()
.filter(|&(_, p)| potential_recursive_statics.contains(p))
.map(|(idx, _)| idx)
.collect();
body.traverse_tree_with(&mut |air_tree: &mut AirTree, _| {
modify_self_calls(air_tree, key, variant, &recursive_static_indexes);
});
if recursive_static_indexes.len() > 0 {
println!("~~ {}: {:?}", key.function_name, recursive_static_indexes.iter().map(|i| func_params[*i].clone()).collect::<Vec<String>>());
}
}
let recursive_nonstatics = func_params.iter().filter(|p| !potential_recursive_statics.contains(p)).cloned().collect();
body = AirTree::define_func( body = AirTree::define_func(
&key.function_name, &key.function_name,
&key.module_name, &key.module_name,
@ -2854,19 +2830,21 @@ impl<'a> CodeGenerator<'a> {
.iter() .iter()
.any(|(key, variant)| &dep_key == key && &dep_variant == variant); .any(|(key, variant)| &dep_key == key && &dep_variant == variant);
if is_dependent_recursive { let recursive_nonstatics = if is_dependent_recursive {
dep_air_tree.traverse_tree_with(&mut |air_tree: &mut AirTree, _| { modify_self_calls(&mut dep_air_tree, &dep_key, &dep_variant, &dependent_params)
modify_self_calls(air_tree, &dep_key, &dep_variant, &vec![]); } else {
}); dependent_params.clone()
} };
dep_insertions.push(AirTree::define_func( dep_insertions.push(AirTree::define_func(
&dep_key.function_name, &dep_key.function_name,
&dep_key.module_name, &dep_key.module_name,
&dep_variant, &dep_variant,
dependent_params.clone(),
is_dependent_recursive,
dependent_params, dependent_params,
is_dependent_recursive,
recursive_nonstatics,
dep_air_tree, dep_air_tree,
)); ));

View File

@ -623,11 +623,6 @@ pub fn identify_recursive_static_params(
AirTree::Expression(AirExpression::Var { name, .. }) => { AirTree::Expression(AirExpression::Var { name, .. }) => {
// "shadowed in an ancestor scope" means "the definition scope is a prefix of our scope" // "shadowed in an ancestor scope" means "the definition scope is a prefix of our scope"
name != param || if let Some(p) = shadowed_parameters.get(param) { name != param || if let Some(p) = shadowed_parameters.get(param) {
println!("param: {:?}", param);
println!("arg: {:?}", arg);
println!("p: {:?}", *p);
println!("tree_path: {:?}", tree_path);
println!("common_ancestor: {:?}", p.common_ancestor(tree_path));
p.common_ancestor(tree_path) == *p p.common_ancestor(tree_path) == *p
} else { } else {
false false
@ -648,7 +643,27 @@ pub fn identify_recursive_static_params(
} }
} }
pub fn modify_self_calls(air_tree: &mut AirTree, func_key: &FunctionAccessKey, variant: &String, static_recursive_params: &Vec<usize>) { pub fn modify_self_calls(body: &mut AirTree, func_key: &FunctionAccessKey, variant: &String, func_params: &Vec<String>) -> Vec<String> {
let mut potential_recursive_statics = func_params.clone();
// identify which parameters are recursively nonstatic (i.e. get modified before the self-call)
// TODO: this would be a lot simpler if each `Var`, `Let`, function argument, etc. had a unique identifier
// rather than just a name; this would let us track if the Var passed to itself was the same value as the method argument
let mut shadowed_parameters: HashMap<String, TreePath> = HashMap::new();
body.traverse_tree_with(&mut |air_tree: &mut AirTree, tree_path| {
identify_recursive_static_params(air_tree, tree_path, &func_params, func_key, variant, &mut shadowed_parameters, &mut potential_recursive_statics);
});
// Find the index of any recursively static parameters,
// so we can remove them from the call-site of each recursive call
let recursive_static_indexes: Vec<_> = func_params
.iter()
.enumerate()
.filter(|&(_, p)| potential_recursive_statics.contains(p))
.map(|(idx, _)| idx)
.collect();
// Modify any self calls to remove recursive static parameters and append `self` as a parameter for the recursion
body.traverse_tree_with(&mut |air_tree: &mut AirTree, _| {
if let AirTree::Expression(AirExpression::Call { func, args, .. }) = air_tree { if let AirTree::Expression(AirExpression::Call { func, args, .. }) = air_tree {
if let AirTree::Expression(AirExpression::Var { if let AirTree::Expression(AirExpression::Var {
constructor: constructor:
@ -667,7 +682,7 @@ pub fn modify_self_calls(air_tree: &mut AirTree, func_key: &FunctionAccessKey, v
// Remove any static-recursive-parameters, because they'll be bound statically // Remove any static-recursive-parameters, because they'll be bound statically
// above the recursive part of the function // above the recursive part of the function
// note: assumes that static_recursive_params is sorted // note: assumes that static_recursive_params is sorted
for arg in static_recursive_params.iter().rev() { for arg in recursive_static_indexes.iter().rev() {
args.remove(*arg); args.remove(*arg);
} }
let mut new_args = vec![func.as_ref().clone()]; let mut new_args = vec![func.as_ref().clone()];
@ -676,6 +691,9 @@ pub fn modify_self_calls(air_tree: &mut AirTree, func_key: &FunctionAccessKey, v
} }
} }
} }
});
let recursive_nonstatics = func_params.iter().filter(|p| !potential_recursive_statics.contains(p)).cloned().collect();
recursive_nonstatics
} }
pub fn pattern_has_conditions(pattern: &TypedPattern) -> bool { pub fn pattern_has_conditions(pattern: &TypedPattern) -> bool {