Skip to content

Commit

Permalink
Avoid pushing zero-sized variables.
Browse files Browse the repository at this point in the history
  • Loading branch information
ilyalesokhin-starkware committed Oct 18, 2023
1 parent 4c64622 commit b0d4d31
Show file tree
Hide file tree
Showing 51 changed files with 62,278 additions and 63,602 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ store_temp<felt252>([0]) -> ([0])
function_call<user@test::bar>([0]) -> ([3])
drop<Unit>([3]) -> ()
struct_construct<Unit>() -> ([4])
store_temp<Unit>([4]) -> ([4])
return([4])

//! > comment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,5 @@ label_test::foo::3:
function_call<user@test::immovable::<(core::felt252, core::felt252)>>([5]) -> ([8])
drop<Tuple<felt252, felt252>>([8]) -> ()
struct_construct<Unit>() -> ([9])
store_temp<Unit>([9]) -> ([9])
return([9])
label_test::foo::2:
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ store_temp<felt252>([7]) -> ([7])
array_append<felt252>([5], [7]) -> ([8])
struct_construct<Unit>() -> ([9])
store_temp<Array<felt252>>([8]) -> ([8])
store_temp<Unit>([9]) -> ([9])
return([8], [9])

//! > ==========================================================================
Expand Down
8 changes: 6 additions & 2 deletions crates/cairo-lang-sierra-generator/src/store_variables/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ impl<'a> AddStoreVariableStatements<'a> {
}

state.register_outputs(
self.db,
results,
branch_signature,
&invocation.args,
Expand All @@ -160,6 +161,7 @@ impl<'a> AddStoreVariableStatements<'a> {
{
let mut state_at_branch = state.clone();
state_at_branch.register_outputs(
self.db,
&branch.results,
&branch_signature,
&invocation.args,
Expand Down Expand Up @@ -291,6 +293,7 @@ impl<'a> AddStoreVariableStatements<'a> {
VarState::TempVar { ty }
}
VarState::LocalVar => VarState::LocalVar,
VarState::ZeroSizedVar => VarState::ZeroSizedVar,
}
}

Expand Down Expand Up @@ -380,6 +383,7 @@ impl<'a> AddStoreVariableStatements<'a> {
}
}
}
VarState::ZeroSizedVar => (true, VarState::ZeroSizedVar),
var_state => {
// Check if this is part of the prefix. If it is, rename instead of adding
// `store_temp`.
Expand Down Expand Up @@ -426,7 +430,7 @@ impl<'a> AddStoreVariableStatements<'a> {
*var_state = self.store_deferred(&mut state.known_stack, var, &info.ty);
}
}
VarState::LocalVar | VarState::Removed => {}
VarState::ZeroSizedVar | VarState::LocalVar | VarState::Removed => {}
}
}
}
Expand Down Expand Up @@ -455,7 +459,7 @@ impl<'a> AddStoreVariableStatements<'a> {
self.store_local(var, &uninitialized_local_var_id.clone(), ty);
*var_state = VarState::LocalVar;
}
VarState::LocalVar | VarState::Removed => {}
VarState::ZeroSizedVar | VarState::LocalVar | VarState::Removed => {}
};
}
}
Expand Down
96 changes: 55 additions & 41 deletions crates/cairo-lang-sierra-generator/src/store_variables/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use cairo_lang_utils::casts::IntoOrPanic;
use cairo_lang_utils::ordered_hash_map::OrderedHashMap;

use super::known_stack::KnownStack;
use crate::db::SierraGenGroup;

/// Represents the known information about a Sierra variable which contains a deferred value.
/// For example, `[ap - 1] + [ap - 2]`.
Expand All @@ -33,11 +34,17 @@ pub enum DeferredVariableKind {
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum VarState {
/// The variable is a temporary variable with the given type.
TempVar { ty: sierra::ids::ConcreteTypeId },
TempVar {
ty: sierra::ids::ConcreteTypeId,
},
/// The variable is deferred with the given DeferredVariableInfo.
Deferred { info: DeferredVariableInfo },
Deferred {
info: DeferredVariableInfo,
},
/// The variable is a local variable.
LocalVar,
// The variable is of size zero.
ZeroSizedVar,
/// The variable was consumed and can no longer be used.
/// This state is used because there is no efficent way of removing variables
/// from [VariablesState::variables] without effecting thier order.
Expand All @@ -58,6 +65,7 @@ impl VariablesState {
/// Clears the stack if needed.
pub fn register_outputs(
&mut self,
db: &dyn SierraGenGroup,
results: &[sierra::ids::VarId],
branch_signature: &BranchSignature,
args: &[sierra::ids::VarId],
Expand All @@ -76,7 +84,7 @@ impl VariablesState {
}

for (var, var_info) in itertools::zip_eq(results, &branch_signature.vars) {
self.register_output(var.clone(), var_info, args, arg_states);
self.register_output(db, var.clone(), var_info, args, arg_states);
}

// Update `known_stack_size`. It is one more than the maximum of the indices in
Expand All @@ -87,58 +95,64 @@ impl VariablesState {
/// Register an output variable of a libfunc in [Self::variables].
fn register_output(
&mut self,
db: &dyn SierraGenGroup,
res: sierra::ids::VarId,
output_info: &OutputVarInfo,
args: &[sierra::ids::VarId],
arg_states: &[VarState],
) {
let mut add_to_known_stack: Option<isize> = None;

let var_state = match &output_info.ref_info {
OutputVarReferenceInfo::Deferred(kind) => VarState::Deferred {
info: DeferredVariableInfo {
ty: output_info.ty.clone(),
kind: match kind {
DeferredOutputKind::Const => DeferredVariableKind::Const,
DeferredOutputKind::AddConst { .. } => DeferredVariableKind::AddConst,
DeferredOutputKind::Generic => DeferredVariableKind::Generic,
let var_state = if db.get_type_info(output_info.ty.clone()).unwrap().zero_sized {
VarState::ZeroSizedVar
} else {
match &output_info.ref_info {
OutputVarReferenceInfo::Deferred(kind) => VarState::Deferred {
info: DeferredVariableInfo {
ty: output_info.ty.clone(),
kind: match kind {
DeferredOutputKind::Const => DeferredVariableKind::Const,
DeferredOutputKind::AddConst { .. } => DeferredVariableKind::AddConst,
DeferredOutputKind::Generic => DeferredVariableKind::Generic,
},
},
},
},
OutputVarReferenceInfo::NewTempVar { idx } => {
add_to_known_stack = Some(idx.into_or_panic::<isize>());
VarState::TempVar { ty: output_info.ty.clone() }
}
OutputVarReferenceInfo::SimpleDerefs => {
VarState::TempVar { ty: output_info.ty.clone() }
}
OutputVarReferenceInfo::SameAsParam { param_idx }
| OutputVarReferenceInfo::PartialParam { param_idx } => {
// Note that the output type may differ from the param type.
let ty = output_info.ty.clone();
match &arg_states[*param_idx] {
VarState::TempVar { .. } => {
// A partial paramater may be smaller than its parent so it can't replace it
// on the stack.
if matches!(
output_info.ref_info,
OutputVarReferenceInfo::SameAsParam { .. }
) {
add_to_known_stack = self.known_stack.get(&args[*param_idx]);
OutputVarReferenceInfo::NewTempVar { idx } => {
add_to_known_stack = Some(idx.into_or_panic::<isize>());
VarState::TempVar { ty: output_info.ty.clone() }
}
OutputVarReferenceInfo::SimpleDerefs => {
VarState::TempVar { ty: output_info.ty.clone() }
}
OutputVarReferenceInfo::SameAsParam { param_idx }
| OutputVarReferenceInfo::PartialParam { param_idx } => {
// Note that the output type may differ from the param type.
let ty = output_info.ty.clone();
match &arg_states[*param_idx] {
VarState::TempVar { .. } => {
// A partial paramater may be smaller than its parent so it can't
// replace it on the stack.
if matches!(
output_info.ref_info,
OutputVarReferenceInfo::SameAsParam { .. }
) {
add_to_known_stack = self.known_stack.get(&args[*param_idx]);
}
VarState::TempVar { ty }
}
VarState::Deferred { info } => VarState::Deferred {
info: DeferredVariableInfo { ty, kind: info.kind },
},
VarState::LocalVar => VarState::LocalVar,
VarState::ZeroSizedVar | VarState::Removed => {
unreachable!("Unexpected var state.")
}
VarState::TempVar { ty }
}
VarState::Deferred { info } => {
VarState::Deferred { info: DeferredVariableInfo { ty, kind: info.kind } }
}
VarState::LocalVar => VarState::LocalVar,
VarState::Removed => {
unreachable!("Unexpected var state.")
}
}
OutputVarReferenceInfo::NewLocalVar => VarState::LocalVar,
}
OutputVarReferenceInfo::NewLocalVar => VarState::LocalVar,
};

self.variables.insert(res.clone(), var_state);

self.known_stack.remove_variable(&res);
Expand Down
Loading

0 comments on commit b0d4d31

Please sign in to comment.