From 57b3bff98daa2e278ae3eee5a472573a2285e7bb Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Fri, 2 Feb 2024 23:07:54 -0800 Subject: [PATCH 01/13] avm2: More advanced optimizer --- core/src/avm2/activation.rs | 4 +- core/src/avm2/op.rs | 2 +- core/src/avm2/verify.rs | 847 +++++++++++++++++++++++++----------- 3 files changed, 608 insertions(+), 245 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index 550d4067f76b..21d1ddad222e 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -1085,8 +1085,8 @@ impl<'a, 'gc> Activation<'a, 'gc> { } } - fn op_push_byte(&mut self, value: u8) -> Result, Error<'gc>> { - self.push_stack(value as i8 as i32); + fn op_push_byte(&mut self, value: i8) -> Result, Error<'gc>> { + self.push_stack(value as i32); Ok(FrameControl::Continue) } diff --git a/core/src/avm2/op.rs b/core/src/avm2/op.rs index 53263958e365..2e041fc21f0f 100644 --- a/core/src/avm2/op.rs +++ b/core/src/avm2/op.rs @@ -250,7 +250,7 @@ pub enum Op { Pop, PopScope, PushByte { - value: u8, + value: i8, }, PushDouble { value: f64, diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index f7d4d32c9033..9ba112721891 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -1,7 +1,9 @@ +use crate::avm2::class::Class; use crate::avm2::error::{ make_error_1025, make_error_1032, make_error_1054, make_error_1107, verify_error, }; use crate::avm2::method::BytecodeMethod; +use crate::avm2::object::ClassObject; use crate::avm2::op::Op; use crate::avm2::property::Property; use crate::avm2::script::TranslationUnit; @@ -9,7 +11,9 @@ use crate::avm2::{Activation, Error}; use gc_arena::GcCell; use std::collections::{HashMap, HashSet}; use swf::avm2::read::Reader; -use swf::avm2::types::{Index, MethodFlags as AbcMethodFlags, Multiname, Op as AbcOp}; +use swf::avm2::types::{ + Index, MethodFlags as AbcMethodFlags, Multiname as AbcMultiname, Op as AbcOp, +}; use swf::error::Error as AbcReadError; pub struct VerifiedMethodInfo { @@ -22,8 +26,8 @@ pub struct Exception { pub to_offset: u32, pub target_offset: u32, - pub variable_name: Index, - pub type_name: Index, + pub variable_name: Index, + pub type_name: Index, } pub fn verify_method<'gc>( @@ -517,6 +521,140 @@ fn verify_code_starting_from<'gc>( Ok(()) } +#[derive(Clone, Copy, Debug)] +enum ValueType<'gc> { + // Either a class, or null. + Class(ClassObject<'gc>), + Int, + Uint, + Number, + Boolean, + Null, + Any, +} + +#[derive(Clone, Debug)] +struct Locals<'gc>(Vec>); + +impl<'gc> Locals<'gc> { + fn new(size: usize) -> Self { + Self(vec![ValueType::Any; size]) + } + + fn set_class_object(&mut self, index: usize, class: ClassObject<'gc>) { + self.0[index] = ValueType::Class(class); + } + + fn set_class(&mut self, index: usize, class: GcCell<'gc, Class<'gc>>) { + // FIXME: Getting the ClassObject this way should be unnecessary + // after the ClassObject refactor + self.0[index] = class + .read() + .class_objects() + .get(0) + .map(|c| ValueType::Class(*c)) + .unwrap_or(ValueType::Any); + } + + fn set_int(&mut self, index: usize) { + self.0[index] = ValueType::Int; + } + + fn set_uint(&mut self, index: usize) { + self.0[index] = ValueType::Uint; + } + + fn set_number(&mut self, index: usize) { + self.0[index] = ValueType::Number; + } + + fn set_boolean(&mut self, index: usize) { + self.0[index] = ValueType::Boolean; + } + + fn set_any(&mut self, index: usize) { + self.0[index] = ValueType::Any; + } + + fn set_null(&mut self, index: usize) { + self.0[index] = ValueType::Null; + } + + fn set(&mut self, index: usize, value: ValueType<'gc>) { + self.0[index] = value; + } + + fn at(&self, index: usize) -> Option> { + self.0.get(index).copied() + } + + fn len(&self) -> usize { + self.0.len() + } +} + +#[derive(Clone, Debug)] +struct Stack<'gc>(Vec>); + +impl<'gc> Stack<'gc> { + fn new() -> Self { + Self(Vec::new()) + } + + fn push_class_object(&mut self, class: ClassObject<'gc>) { + self.0.push(ValueType::Class(class)); + } + + fn push_class(&mut self, class: GcCell<'gc, Class<'gc>>) { + // FIXME: Getting the ClassObject this way should be unnecessary + // after the ClassObject refactor + self.0.push( + class + .read() + .class_objects() + .get(0) + .map(|c| ValueType::Class(*c)) + .unwrap_or(ValueType::Any), + ); + } + + fn push_int(&mut self) { + self.0.push(ValueType::Int); + } + + fn push_uint(&mut self) { + self.0.push(ValueType::Uint); + } + + fn push_number(&mut self) { + self.0.push(ValueType::Number); + } + + fn push_boolean(&mut self) { + self.0.push(ValueType::Boolean); + } + + fn push_any(&mut self) { + self.0.push(ValueType::Any); + } + + fn push_null(&mut self) { + self.0.push(ValueType::Null); + } + + fn push(&mut self, value: ValueType<'gc>) { + self.0.push(value); + } + + fn pop(&mut self) -> Option> { + self.0.pop() + } + + fn clear(&mut self) { + self.0 = Vec::new(); + } +} + fn optimize<'gc>( activation: &mut Activation<'_, 'gc>, method: &BytecodeMethod<'gc>, @@ -527,6 +665,15 @@ fn optimize<'gc>( #![allow(clippy::manual_filter)] #![allow(clippy::single_match)] + let mut output = crate::string::WString::new(); + activation + .avm2() + .call_stack() + .read() + .clone() + .display(&mut output); + println!("beginning optimizing, call stack: {}", output); + let method_body = method .body() .expect("Cannot verify non-native method without body!"); @@ -546,8 +693,10 @@ fn optimize<'gc>( }; // Initial set of local types - let mut local_types = vec![None; method_body.num_locals as usize]; - local_types[0] = this_class; + let mut initial_local_types = Locals::new(method_body.num_locals as usize); + if let Some(this_class) = this_class { + initial_local_types.set_class_object(0, this_class); + } // Logic to only allow for type-based optimizations on types that // we're absolutely sure about- invalidate the local register's @@ -561,280 +710,494 @@ fn optimize<'gc>( | Op::IncLocalI { index } | Op::DecLocal { index } | Op::DecLocalI { index } => { - if (*index as usize) < local_types.len() { - local_types[*index as usize] = None; + if (*index as usize) < initial_local_types.len() { + initial_local_types.set_any(*index as usize); } } Op::HasNext2 { object_register, index_register, } => { - if (*object_register as usize) < local_types.len() { - local_types[*object_register as usize] = None; + if (*object_register as usize) < initial_local_types.len() { + initial_local_types.set_any(*object_register as usize); } - if (*index_register as usize) < local_types.len() { - local_types[*index_register as usize] = None; + if (*index_register as usize) < initial_local_types.len() { + initial_local_types.set_any(*index_register as usize); } } _ => {} } } - let mut previous_op = None; + let mut stack = Stack::new(); + let mut local_types = initial_local_types.clone(); + for (i, op) in code.iter_mut().enumerate() { - if let Some(previous_op_some) = previous_op { - if !jump_targets.contains(&(i as i32)) { - match op { - Op::CoerceB => match previous_op_some { - Op::CoerceB - | Op::Equals - | Op::GreaterEquals - | Op::GreaterThan - | Op::LessEquals - | Op::LessThan - | Op::PushTrue - | Op::PushFalse - | Op::Not - | Op::IsType { .. } - | Op::IsTypeLate => { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - _ => {} - }, - Op::CoerceD => match previous_op_some { - Op::CoerceD - | Op::PushDouble { .. } - | Op::Multiply - | Op::Divide - | Op::Modulo - | Op::Increment - | Op::Decrement - | Op::Negate => { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - _ => {} - }, - Op::CoerceI => match previous_op_some { - Op::CoerceI | Op::PushByte { .. } | Op::PushShort { .. } => { - previous_op = Some(op.clone()); + if jump_targets.contains(&(i as i32)) { + stack.clear(); + local_types = initial_local_types.clone(); + } + + match op { + Op::CoerceB => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Boolean)) { + println!(" optimized a CoerceB to Nop - idx {}", i); + *op = Op::Nop; + } + stack.push_boolean(); + } + Op::CoerceD => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Number)) { + println!(" optimized a CoerceD to Nop - idx {}", i); + *op = Op::Nop; + } + stack.push_number(); + } + Op::CoerceI => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Int)) + || matches!(stack_value, Some(ValueType::Uint)) + { + println!(" optimized a CoerceI to Nop - idx {}", i); + *op = Op::Nop; + } + stack.push_int(); + } + Op::CoerceU => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Uint)) { + println!(" optimized a CoerceU to Nop - idx {}", i); + *op = Op::Nop; + } + stack.push_uint(); + } + Op::CoerceA => { + stack.pop(); + stack.push_any(); + } + Op::Equals | Op::LessEquals | Op::LessThan | Op::GreaterThan | Op::GreaterEquals => { + stack.pop(); + stack.pop(); + stack.push_boolean(); + } + Op::Not => { + stack.pop(); + stack.push_boolean(); + } + Op::PushTrue | Op::PushFalse => { + stack.push_boolean(); + } + Op::PushNull => { + stack.push_null(); + } + Op::PushUndefined => { + stack.push_any(); + } + Op::PushNaN => { + stack.push_number(); + } + Op::PushByte { value } => { + if *value >= 0 { + stack.push_uint(); + } else { + stack.push_int(); + } + } + Op::PushShort { value } => { + if *value >= 0 { + stack.push_uint(); + } else { + stack.push_int(); + } + } + Op::DecrementI => { + // This doesn't give any Number-int guarantees + stack.pop(); + stack.push_any(); + } + Op::IncrementI => { + // This doesn't give any Number-int guarantees + stack.pop(); + stack.push_any(); + } + Op::DecLocalI { index } => { + if (*index as usize) < local_types.len() { + // This doesn't give any Number-int guarantees + local_types.set_any(*index as usize); + } + } + Op::IncLocalI { index } => { + if (*index as usize) < local_types.len() { + // This doesn't give any Number-int guarantees + local_types.set_any(*index as usize); + } + } + Op::Add => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::Subtract => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::Multiply => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::BitNot => { + stack.pop(); + stack.push_any(); + } + Op::BitAnd => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::PushDouble { .. } => { + stack.push_number(); + } + Op::PushString { .. } => { + stack.push_class_object(activation.avm2().classes().string); + } + Op::NewArray { num_args } => { + for _ in 0..num_args { + stack.pop(); + } + + stack.push_class_object(activation.avm2().classes().array); + } + Op::NewFunction { .. } => { + stack.push_class_object(activation.avm2().classes().function); + } + Op::IsType { .. } => { + stack.pop(); + stack.push_boolean(); + } + Op::IsTypeLate => { + stack.pop(); + stack.pop(); + stack.push_boolean(); + } + Op::AsType { + type_name: name_index, + } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + + let resolved_type = if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + activation + .domain() + .get_class(&multiname, activation.context.gc_context) + } else { + None + } + } else { + None + }; + + let stack_value = stack.pop(); + if resolved_type.is_some() { + if matches!(stack_value, Some(ValueType::Null)) { + println!(" optimized an AsType to Nop - idx {}", i); + *op = Op::Nop; + } + } + + if let Some(resolved_type) = resolved_type { + stack.push_class(resolved_type); + } else { + stack.push_any(); + } + } + Op::Coerce { index: name_index } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + + let resolved_type = if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + activation + .domain() + .get_class(&multiname, activation.context.gc_context) + } else { + None + } + } else { + None + }; + + let stack_value = stack.pop(); + if let Some(resolved_type) = resolved_type { + if matches!(stack_value, Some(ValueType::Null)) { + // As long as this Coerce isn't coercing to one + // of these special classes, we can remove it. + if !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().int.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().uint.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().number.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().boolean.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().void.inner_class_definition(), + ) { + println!(" optimized a Coerce to Nop (Null case) - idx {}", i); *op = Op::Nop; - continue; - } - Op::PushInt { value } => { - if value >= -(1 << 28) && value < (1 << 28) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } } - _ => {} - }, - Op::CoerceU => match previous_op_some { - Op::CoerceU => { - previous_op = Some(op.clone()); + } else if let Some(ValueType::Class(class_object)) = stack_value { + if GcCell::ptr_eq(resolved_type, class_object.inner_class_definition()) { + println!( + " optimized a Coerce to Nop (type: {:?}) - idx {}", + class_object, i + ); *op = Op::Nop; - continue; - } - Op::PushByte { value } => { - if (value as i8) >= 0 { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } } - Op::PushShort { value } => { - if value >= 0 { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - Op::PushInt { value } => { - if value >= 0 && value < (1 << 28) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - _ => {} - }, - Op::GetProperty { index: name_index } => match previous_op_some { - Op::GetLocal { index: local_index } => { - let class = local_types[local_index as usize]; - if let Some(class) = class { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname( - *name_index, - &mut activation.context, - ); + } + + stack.push_class(resolved_type); + } else { + stack.push_any(); + } + } + Op::PushScope => { + stack.pop(); + } + Op::Pop => { + stack.pop(); + } + Op::Dup => { + let stack_value = stack.pop(); + if let Some(stack_value) = stack_value { + stack.push(stack_value); + stack.push(stack_value); + } + } + Op::SetLocal { index } => { + let stack_value = stack.pop(); + if (*index as usize) < local_types.len() { + if let Some(stack_value) = stack_value { + local_types.set(*index as usize, stack_value); + } else { + local_types.set_any(*index as usize); + } + } + } + Op::GetLocal { index } => { + let local_type = local_types.at(*index as usize); + if let Some(local_type) = local_type { + stack.push(local_type); + } else { + stack.push_any(); + } + } + Op::GetLex { .. } => { + stack.push_any(); + } + Op::FindPropStrict { .. } => { + // Avoid handling for now + stack.clear(); + } + Op::FindProperty { .. } => { + // Avoid handling for now + stack.clear(); + } + Op::GetProperty { index: name_index } => { + let stack_value = stack.pop(); + if let Some(ValueType::Class(class)) = stack_value { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) - | Some(Property::ConstSlot { slot_id }) => { - previous_op = Some(op.clone()); - *op = Op::GetSlot { index: slot_id }; - continue; - } - Some(Property::Virtual { get: Some(get), .. }) => { - previous_op = Some(op.clone()); - *op = Op::CallMethod { - num_args: 0, - index: Index::new(get), - }; - continue; - } - _ => {} - } - } + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) + | Some(Property::ConstSlot { slot_id }) => { + println!(" optimized a GetProperty to GetSlot - idx {}", i); + *op = Op::GetSlot { index: slot_id }; } + Some(Property::Virtual { get: Some(get), .. }) => { + println!( + " optimized a GetProperty to CallMethod - idx {}", + i + ); + *op = Op::CallMethod { + num_args: 0, + index: Index::new(get), + }; + } + _ => {} } + } else { + // Avoid handling lazy for now + stack.clear(); } - _ => {} - }, - Op::AsType { - type_name: name_index, - } => { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname( - *name_index, - &mut activation.context, - ); + } + } + stack.push_any(); + } + Op::InitProperty { index: name_index } => { + stack.pop(); + let stack_value = stack.pop(); + if let Some(ValueType::Class(class)) = stack_value { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - let resolved_type = if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - activation - .domain() - .get_class(&multiname, activation.context.gc_context) - } else { - None - } - } else { - None - }; - - if resolved_type.is_some() { - match previous_op_some { - Op::PushNull => { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) + | Some(Property::ConstSlot { slot_id }) => { + println!( + " optimized an InitProperty to SetSlot - idx {}", + i + ); + *op = Op::SetSlot { index: slot_id }; } _ => {} } + } else { + // Avoid handling lazy for now + stack.clear(); } } - Op::Coerce { index: name_index } => { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname( - *name_index, - &mut activation.context, - ); + } + } + Op::SetProperty { index: name_index } => { + stack.pop(); + let stack_value = stack.pop(); + if let Some(ValueType::Class(class)) = stack_value { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - let resolved_type = if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - activation - .domain() - .get_class(&multiname, activation.context.gc_context) - } else { - None - } - } else { - None - }; - - if let Some(class) = resolved_type { - match previous_op_some { - Op::PushNull => { - // As long as this Coerce isn't coercing to one - // of these special classes, we can remove it. - if !GcCell::ptr_eq( - class, - activation.avm2().classes().int.inner_class_definition(), - ) && !GcCell::ptr_eq( - class, - activation.avm2().classes().uint.inner_class_definition(), - ) && !GcCell::ptr_eq( - class, - activation.avm2().classes().number.inner_class_definition(), - ) && !GcCell::ptr_eq( - class, - activation - .avm2() - .classes() - .boolean - .inner_class_definition(), - ) && !GcCell::ptr_eq( - class, - activation.avm2().classes().void.inner_class_definition(), - ) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - Op::PushString { .. } => { - if GcCell::ptr_eq( - class, - activation.avm2().classes().string.inner_class_definition(), - ) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - Op::NewArray { .. } => { - if GcCell::ptr_eq( - class, - activation.avm2().classes().array.inner_class_definition(), - ) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - Op::NewFunction { .. } => { - if GcCell::ptr_eq( - class, - activation - .avm2() - .classes() - .function - .inner_class_definition(), - ) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - Op::Coerce { - index: previous_name_index, - } => { - if name_index.as_u30() == previous_name_index.as_u30() { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) => { + println!(" optimized a SetProperty to SetSlot - idx {}", i); + *op = Op::SetSlot { index: slot_id }; } _ => {} } + } else { + // Avoid handling lazy for now + stack.clear(); } } - _ => {} } } - } + Op::CallProperty { + index: name_index, + num_args, + } => { + // Args... + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if multiname.has_lazy_component() { + // Avoid handling lazy for now + stack.clear(); + } + } - previous_op = Some(op.clone()); + // Avoid checking return value for now + stack.push_any(); + } + Op::CallPropVoid { .. } => { + // Avoid handling for now + stack.clear(); + } + Op::Nop => {} + Op::IfTrue { .. } | Op::IfFalse { .. } => { + stack.pop(); + } + Op::IfStrictEq { .. } + | Op::IfStrictNe { .. } + | Op::IfEq { .. } + | Op::IfNe { .. } + | Op::IfGe { .. } + | Op::IfGt { .. } + | Op::IfLe { .. } + | Op::IfLt { .. } + | Op::IfNge { .. } + | Op::IfNgt { .. } + | Op::IfNle { .. } + | Op::IfNlt { .. } => { + stack.pop(); + stack.pop(); + } + _ => { + stack.clear(); + local_types = initial_local_types.clone(); + } + } /* + + if let Some(previous_op_some) = previous_op { + if !jump_targets.contains(&(i as i32)) { + match op { + Op::CoerceD => match previous_op_some { + Op::Multiply + | Op::Divide + | Op::Modulo + | Op::Increment + | Op::Decrement + | Op::Negate => { + previous_op = Some(op.clone()); + *op = Op::Nop; + continue; + } + _ => {} + }, + Op::CoerceI => match previous_op_some { + Op::PushInt { value } => { + if value >= -(1 << 28) && value < (1 << 28) { + previous_op = Some(op.clone()); + *op = Op::Nop; + continue; + } + } + _ => {} + }, + Op::CoerceU => match previous_op_some { + Op::PushInt { value } => { + if value >= 0 && value < (1 << 28) { + previous_op = Some(op.clone()); + *op = Op::Nop; + continue; + } + } + _ => {} + }, + _ => {} + } + } + }*/ } + + println!(); } fn ops_can_throw_error(ops: &[AbcOp]) -> bool { @@ -944,7 +1307,7 @@ fn resolve_op<'gc>( op: AbcOp, ) -> Result> { Ok(match op { - AbcOp::PushByte { value } => Op::PushByte { value }, + AbcOp::PushByte { value } => Op::PushByte { value: value as i8 }, AbcOp::PushDouble { value } => { let value = pool_double(activation, translation_unit, value)?; From aff47f1623bc3d76daa316d0b58b2ecd18e366f8 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 3 Feb 2024 08:35:35 -0800 Subject: [PATCH 02/13] avm2: Handle more ops in abstract optimizer --- core/src/avm2/verify.rs | 186 ++++++++++++++++++++++++++++++++-------- 1 file changed, 152 insertions(+), 34 deletions(-) diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 9ba112721891..98bac87b15e4 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -834,6 +834,21 @@ fn optimize<'gc>( local_types.set_any(*index as usize); } } + Op::AddI => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::SubtractI => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::MultiplyI => { + stack.pop(); + stack.pop(); + stack.push_any(); + } Op::Add => { stack.pop(); stack.pop(); @@ -849,6 +864,11 @@ fn optimize<'gc>( stack.pop(); stack.push_any(); } + Op::Modulo => { + stack.pop(); + stack.pop(); + stack.push_any(); + } Op::BitNot => { stack.pop(); stack.push_any(); @@ -858,6 +878,16 @@ fn optimize<'gc>( stack.pop(); stack.push_any(); } + Op::BitOr => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::BitXor => { + stack.pop(); + stack.pop(); + stack.push_any(); + } Op::PushDouble { .. } => { stack.push_number(); } @@ -883,6 +913,15 @@ fn optimize<'gc>( stack.pop(); stack.push_boolean(); } + Op::ApplyType { num_types } => { + for _ in 0..*num_types { + stack.pop(); + } + + stack.pop(); + + stack.push_any(); + } Op::AsType { type_name: name_index, } => { @@ -1006,9 +1045,19 @@ fn optimize<'gc>( Op::GetLex { .. } => { stack.push_any(); } - Op::FindPropStrict { .. } => { - // Avoid handling for now - stack.clear(); + Op::FindPropStrict { index: name_index } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + stack.push_any(); + } else { + // Avoid handling lazy for now + stack.clear(); + } + } } Op::FindProperty { .. } => { // Avoid handling for now @@ -1023,23 +1072,28 @@ fn optimize<'gc>( if let Ok(multiname) = multiname { if !multiname.has_lazy_component() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) - | Some(Property::ConstSlot { slot_id }) => { - println!(" optimized a GetProperty to GetSlot - idx {}", i); - *op = Op::GetSlot { index: slot_id }; + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) + | Some(Property::ConstSlot { slot_id }) => { + println!( + " optimized a GetProperty to GetSlot - idx {}", + i + ); + *op = Op::GetSlot { index: slot_id }; + } + Some(Property::Virtual { get: Some(get), .. }) => { + println!( + " optimized a GetProperty to CallMethod - idx {}", + i + ); + *op = Op::CallMethod { + num_args: 0, + index: Index::new(get), + }; + } + _ => {} } - Some(Property::Virtual { get: Some(get), .. }) => { - println!( - " optimized a GetProperty to CallMethod - idx {}", - i - ); - *op = Op::CallMethod { - num_args: 0, - index: Index::new(get), - }; - } - _ => {} } } else { // Avoid handling lazy for now @@ -1059,16 +1113,18 @@ fn optimize<'gc>( if let Ok(multiname) = multiname { if !multiname.has_lazy_component() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) - | Some(Property::ConstSlot { slot_id }) => { - println!( - " optimized an InitProperty to SetSlot - idx {}", - i - ); - *op = Op::SetSlot { index: slot_id }; + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) + | Some(Property::ConstSlot { slot_id }) => { + println!( + " optimized an InitProperty to SetSlot - idx {}", + i + ); + *op = Op::SetSlot { index: slot_id }; + } + _ => {} } - _ => {} } } else { // Avoid handling lazy for now @@ -1087,12 +1143,17 @@ fn optimize<'gc>( if let Ok(multiname) = multiname { if !multiname.has_lazy_component() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) => { - println!(" optimized a SetProperty to SetSlot - idx {}", i); - *op = Op::SetSlot { index: slot_id }; + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) => { + println!( + " optimized a SetProperty to SetSlot - idx {}", + i + ); + *op = Op::SetSlot { index: slot_id }; + } + _ => {} } - _ => {} } } else { // Avoid handling lazy for now @@ -1101,11 +1162,57 @@ fn optimize<'gc>( } } } + Op::Construct { num_args } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + stack.pop(); + + // Avoid checking return value for now + stack.push_any(); + } + Op::ConstructSuper { num_args } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + stack.pop(); + } + Op::ConstructProp { + index: name_index, + num_args, + } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if multiname.has_lazy_component() { + // Avoid handling lazy for now + stack.clear(); + } + } + + // Avoid checking return value for now + stack.push_any(); + } Op::CallProperty { index: name_index, num_args, } => { - // Args... + // Arguments for _ in 0..*num_args { stack.pop(); } @@ -1131,6 +1238,9 @@ fn optimize<'gc>( stack.clear(); } Op::Nop => {} + Op::DebugFile { .. } => {} + Op::DebugLine { .. } => {} + Op::Debug { .. } => {} Op::IfTrue { .. } | Op::IfFalse { .. } => { stack.pop(); } @@ -1149,6 +1259,14 @@ fn optimize<'gc>( stack.pop(); stack.pop(); } + Op::Si8 => { + stack.pop(); + stack.pop(); + } + Op::Li8 => { + stack.pop(); + stack.push_int(); + } _ => { stack.clear(); local_types = initial_local_types.clone(); From 45c793a090d9f73e2c9cd6697a45fb478c01a9f2 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 3 Feb 2024 11:24:51 -0800 Subject: [PATCH 03/13] avm2: Support type inference for chained `GetProperty`s --- core/src/avm2/property.rs | 31 +++++- core/src/avm2/verify.rs | 198 +++++++++++++++++++------------------- core/src/avm2/vtable.rs | 8 ++ 3 files changed, 136 insertions(+), 101 deletions(-) diff --git a/core/src/avm2/property.rs b/core/src/avm2/property.rs index c7981b61ad0d..374994fed514 100644 --- a/core/src/avm2/property.rs +++ b/core/src/avm2/property.rs @@ -33,7 +33,7 @@ pub enum Property { /// Additionally, property class resolution uses special /// logic, different from normal "runtime" class resolution, /// that allows private types to be referenced. -#[derive(Collect, Clone)] +#[derive(Collect, Clone, Copy)] #[collect(no_drop)] pub enum PropertyClass<'gc> { /// The type `*` (Multiname::is_any()). This allows @@ -100,6 +100,35 @@ impl<'gc> PropertyClass<'gc> { } } + pub fn get_class( + &mut self, + activation: &mut Activation<'_, 'gc>, + ) -> Result>>, Error<'gc>> { + match self { + PropertyClass::Class(class) => Ok(Some(*class)), + PropertyClass::Name(gc) => { + let (name, unit) = &**gc; + if name.is_any_name() { + *self = PropertyClass::Any; + Ok(None) + } else { + let domain = + unit.map_or(activation.avm2().playerglobals_domain, |u| u.domain()); + if let Some(class) = domain.get_class(name, activation.context.gc_context) { + *self = PropertyClass::Class(class); + Ok(Some(class)) + } else { + Err( + format!("Could not resolve class {name:?} for property class lookup") + .into(), + ) + } + } + } + PropertyClass::Any => Ok(None), + } + } + pub fn get_name(&self, mc: &Mutation<'gc>) -> Multiname<'gc> { match self { PropertyClass::Class(class) => class.read().name().into(), diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 98bac87b15e4..651475147258 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -742,7 +742,6 @@ fn optimize<'gc>( Op::CoerceB => { let stack_value = stack.pop(); if matches!(stack_value, Some(ValueType::Boolean)) { - println!(" optimized a CoerceB to Nop - idx {}", i); *op = Op::Nop; } stack.push_boolean(); @@ -750,7 +749,6 @@ fn optimize<'gc>( Op::CoerceD => { let stack_value = stack.pop(); if matches!(stack_value, Some(ValueType::Number)) { - println!(" optimized a CoerceD to Nop - idx {}", i); *op = Op::Nop; } stack.push_number(); @@ -760,7 +758,6 @@ fn optimize<'gc>( if matches!(stack_value, Some(ValueType::Int)) || matches!(stack_value, Some(ValueType::Uint)) { - println!(" optimized a CoerceI to Nop - idx {}", i); *op = Op::Nop; } stack.push_int(); @@ -768,7 +765,6 @@ fn optimize<'gc>( Op::CoerceU => { let stack_value = stack.pop(); if matches!(stack_value, Some(ValueType::Uint)) { - println!(" optimized a CoerceU to Nop - idx {}", i); *op = Op::Nop; } stack.push_uint(); @@ -777,6 +773,13 @@ fn optimize<'gc>( stack.pop(); stack.push_any(); } + Op::CoerceS => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Null)) { + *op = Op::Nop; + } + stack.push_class_object(activation.avm2().classes().string); + } Op::Equals | Op::LessEquals | Op::LessThan | Op::GreaterThan | Op::GreaterEquals => { stack.pop(); stack.pop(); @@ -812,6 +815,15 @@ fn optimize<'gc>( stack.push_int(); } } + Op::PushInt { value } => { + if *value < -(1 << 28) || *value >= (1 << 28) { + stack.push_number(); + } else if *value >= 0 { + stack.push_uint(); + } else { + stack.push_int(); + } + } Op::DecrementI => { // This doesn't give any Number-int guarantees stack.pop(); @@ -834,6 +846,18 @@ fn optimize<'gc>( local_types.set_any(*index as usize); } } + Op::Increment => { + stack.pop(); + stack.push_number(); + } + Op::Decrement => { + stack.pop(); + stack.push_number(); + } + Op::Negate => { + stack.pop(); + stack.push_number(); + } Op::AddI => { stack.pop(); stack.pop(); @@ -862,12 +886,26 @@ fn optimize<'gc>( Op::Multiply => { stack.pop(); stack.pop(); - stack.push_any(); + + // NOTE: In our current implementation, this is guaranteed, + // but it may not be after correctness fixes to match avmplus + stack.push_number(); + } + Op::Divide => { + stack.pop(); + stack.pop(); + + // NOTE: In our current implementation, this is guaranteed, + // but it may not be after correctness fixes to match avmplus + stack.push_number(); } Op::Modulo => { stack.pop(); stack.pop(); - stack.push_any(); + + // NOTE: In our current implementation, this is guaranteed, + // but it may not be after correctness fixes to match avmplus + stack.push_number(); } Op::BitNot => { stack.pop(); @@ -944,7 +982,6 @@ fn optimize<'gc>( let stack_value = stack.pop(); if resolved_type.is_some() { if matches!(stack_value, Some(ValueType::Null)) { - println!(" optimized an AsType to Nop - idx {}", i); *op = Op::Nop; } } @@ -993,15 +1030,10 @@ fn optimize<'gc>( resolved_type, activation.avm2().classes().void.inner_class_definition(), ) { - println!(" optimized a Coerce to Nop (Null case) - idx {}", i); *op = Op::Nop; } } else if let Some(ValueType::Class(class_object)) = stack_value { if GcCell::ptr_eq(resolved_type, class_object.inner_class_definition()) { - println!( - " optimized a Coerce to Nop (type: {:?}) - idx {}", - class_object, i - ); *op = Op::Nop; } } @@ -1064,29 +1096,43 @@ fn optimize<'gc>( stack.clear(); } Op::GetProperty { index: name_index } => { + let mut stack_push_done = false; let stack_value = stack.pop(); - if let Some(ValueType::Class(class)) = stack_value { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { match class.instance_vtable().get_trait(&multiname) { Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => { - println!( - " optimized a GetProperty to GetSlot - idx {}", - i - ); *op = Op::GetSlot { index: slot_id }; + + let mut value_class = + class.instance_vtable().slot_classes() + [slot_id as usize]; + let resolved_value_class = + value_class.get_class(activation); + if let Ok(class) = resolved_value_class { + stack_push_done = true; + + if let Some(class) = class { + stack.push_class(class); + } else { + stack.push_any(); + } + } + + class.instance_vtable().set_slot_class( + activation.context.gc_context, + slot_id as usize, + value_class, + ); } Some(Property::Virtual { get: Some(get), .. }) => { - println!( - " optimized a GetProperty to CallMethod - idx {}", - i - ); *op = Op::CallMethod { num_args: 0, index: Index::new(get), @@ -1095,70 +1141,65 @@ fn optimize<'gc>( _ => {} } } - } else { - // Avoid handling lazy for now - stack.clear(); } + } else { + // Avoid handling lazy for now + stack.clear(); } } - stack.push_any(); + + if !stack_push_done { + stack.push_any(); + } } Op::InitProperty { index: name_index } => { stack.pop(); let stack_value = stack.pop(); - if let Some(ValueType::Class(class)) = stack_value { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { match class.instance_vtable().get_trait(&multiname) { Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => { - println!( - " optimized an InitProperty to SetSlot - idx {}", - i - ); *op = Op::SetSlot { index: slot_id }; } _ => {} } } - } else { - // Avoid handling lazy for now - stack.clear(); } + } else { + // Avoid handling lazy for now + stack.clear(); } } } Op::SetProperty { index: name_index } => { stack.pop(); let stack_value = stack.pop(); - if let Some(ValueType::Class(class)) = stack_value { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { match class.instance_vtable().get_trait(&multiname) { Some(Property::Slot { slot_id }) => { - println!( - " optimized a SetProperty to SetSlot - idx {}", - i - ); *op = Op::SetSlot { index: slot_id }; } _ => {} } } - } else { - // Avoid handling lazy for now - stack.clear(); } + } else { + // Avoid handling lazy for now + stack.clear(); } } } @@ -1271,51 +1312,8 @@ fn optimize<'gc>( stack.clear(); local_types = initial_local_types.clone(); } - } /* - - if let Some(previous_op_some) = previous_op { - if !jump_targets.contains(&(i as i32)) { - match op { - Op::CoerceD => match previous_op_some { - Op::Multiply - | Op::Divide - | Op::Modulo - | Op::Increment - | Op::Decrement - | Op::Negate => { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - _ => {} - }, - Op::CoerceI => match previous_op_some { - Op::PushInt { value } => { - if value >= -(1 << 28) && value < (1 << 28) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - _ => {} - }, - Op::CoerceU => match previous_op_some { - Op::PushInt { value } => { - if value >= 0 && value < (1 << 28) { - previous_op = Some(op.clone()); - *op = Op::Nop; - continue; - } - } - _ => {} - }, - _ => {} - } - } - }*/ + } } - - println!(); } fn ops_can_throw_error(ops: &[AbcOp]) -> bool { diff --git a/core/src/avm2/vtable.rs b/core/src/avm2/vtable.rs index 08270b6e31d6..49934090149c 100644 --- a/core/src/avm2/vtable.rs +++ b/core/src/avm2/vtable.rs @@ -204,6 +204,14 @@ impl<'gc> VTable<'gc> { Ref::map(self.0.read(), |v| &v.default_slots) } + pub fn slot_classes(&self) -> Ref>> { + Ref::map(self.0.read(), |v| &v.slot_classes) + } + + pub fn set_slot_class(&self, mc: &Mutation<'gc>, index: usize, value: PropertyClass<'gc>) { + self.0.write(mc).slot_classes[index] = value; + } + /// Calculate the flattened list of instance traits that this class /// maintains. /// From 951f5017659b2673c17ab84eb44f8b72e770b1e1 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 3 Feb 2024 13:05:15 -0800 Subject: [PATCH 04/13] chore: cleanup --- core/src/avm2/verify.rs | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 651475147258..20aaa13117e3 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -545,41 +545,10 @@ impl<'gc> Locals<'gc> { self.0[index] = ValueType::Class(class); } - fn set_class(&mut self, index: usize, class: GcCell<'gc, Class<'gc>>) { - // FIXME: Getting the ClassObject this way should be unnecessary - // after the ClassObject refactor - self.0[index] = class - .read() - .class_objects() - .get(0) - .map(|c| ValueType::Class(*c)) - .unwrap_or(ValueType::Any); - } - - fn set_int(&mut self, index: usize) { - self.0[index] = ValueType::Int; - } - - fn set_uint(&mut self, index: usize) { - self.0[index] = ValueType::Uint; - } - - fn set_number(&mut self, index: usize) { - self.0[index] = ValueType::Number; - } - - fn set_boolean(&mut self, index: usize) { - self.0[index] = ValueType::Boolean; - } - fn set_any(&mut self, index: usize) { self.0[index] = ValueType::Any; } - fn set_null(&mut self, index: usize) { - self.0[index] = ValueType::Null; - } - fn set(&mut self, index: usize, value: ValueType<'gc>) { self.0[index] = value; } @@ -665,15 +634,6 @@ fn optimize<'gc>( #![allow(clippy::manual_filter)] #![allow(clippy::single_match)] - let mut output = crate::string::WString::new(); - activation - .avm2() - .call_stack() - .read() - .clone() - .display(&mut output); - println!("beginning optimizing, call stack: {}", output); - let method_body = method .body() .expect("Cannot verify non-native method without body!"); From 781de6fd53997f80bc771217fb4276a8d7b2190f Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 3 Feb 2024 15:17:50 -0800 Subject: [PATCH 05/13] chore: appease clippy --- core/src/avm2/verify.rs | 6 ++---- core/src/avm2/vtable.rs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 20aaa13117e3..5f19b5ea1317 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -940,10 +940,8 @@ fn optimize<'gc>( }; let stack_value = stack.pop(); - if resolved_type.is_some() { - if matches!(stack_value, Some(ValueType::Null)) { - *op = Op::Nop; - } + if resolved_type.is_some() && matches!(stack_value, Some(ValueType::Null)) { + *op = Op::Nop; } if let Some(resolved_type) = resolved_type { diff --git a/core/src/avm2/vtable.rs b/core/src/avm2/vtable.rs index 49934090149c..a8a41cd3d646 100644 --- a/core/src/avm2/vtable.rs +++ b/core/src/avm2/vtable.rs @@ -167,7 +167,7 @@ impl<'gc> VTable<'gc> { activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { // Drop the `write()` guard, as 'slot_class.coerce' may need to access this vtable. - let mut slot_class = { self.0.read().slot_classes[slot_id as usize].clone() }; + let mut slot_class = { self.0.read().slot_classes[slot_id as usize] }; let (value, changed) = slot_class.coerce(activation, value)?; From 54d483b8fc5867038fecf09cb194e4ec8a2d746f Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 3 Feb 2024 19:56:41 -0800 Subject: [PATCH 06/13] avm2: Add more ops to optimizer and let it use resolved argument types --- core/src/avm2/verify.rs | 130 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 126 insertions(+), 4 deletions(-) diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 5f19b5ea1317..1c744af9d38c 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -545,6 +545,17 @@ impl<'gc> Locals<'gc> { self.0[index] = ValueType::Class(class); } + fn set_class(&mut self, index: usize, class: GcCell<'gc, Class<'gc>>) { + // FIXME: Getting the ClassObject this way should be unnecessary + // after the ClassObject refactor + self.0[index] = class + .read() + .class_objects() + .get(0) + .map(|c| ValueType::Class(*c)) + .unwrap_or(ValueType::Any); + } + fn set_any(&mut self, index: usize) { self.0[index] = ValueType::Any; } @@ -652,12 +663,37 @@ fn optimize<'gc>( None }; + // TODO: Store these argument types somewhere on the function so they don't + // have to be re-resolved every function call + let mut argument_types = Vec::new(); + for argument in &method.signature { + let type_name = &argument.param_type_name; + + let argument_type = if !type_name.has_lazy_component() { + activation + .domain() + .get_class(type_name, activation.context.gc_context) + } else { + None + }; + + argument_types.push(argument_type); + } + // Initial set of local types let mut initial_local_types = Locals::new(method_body.num_locals as usize); if let Some(this_class) = this_class { initial_local_types.set_class_object(0, this_class); } + let mut i = 1; + for argument_type in argument_types { + if let Some(argument_type) = argument_type { + initial_local_types.set_class(i, argument_type); + } + i += 1; + } + // Logic to only allow for type-based optimizations on types that // we're absolutely sure about- invalidate the local register's // known type if any other register-modifying opcodes mention them @@ -690,11 +726,13 @@ fn optimize<'gc>( } let mut stack = Stack::new(); + let mut scope_stack = Stack::new(); let mut local_types = initial_local_types.clone(); for (i, op) in code.iter_mut().enumerate() { if jump_targets.contains(&(i as i32)) { stack.clear(); + scope_stack.clear(); local_types = initial_local_types.clone(); } @@ -740,7 +778,12 @@ fn optimize<'gc>( } stack.push_class_object(activation.avm2().classes().string); } - Op::Equals | Op::LessEquals | Op::LessThan | Op::GreaterThan | Op::GreaterEquals => { + Op::Equals + | Op::StrictEquals + | Op::LessEquals + | Op::LessThan + | Op::GreaterThan + | Op::GreaterEquals => { stack.pop(); stack.pop(); stack.push_boolean(); @@ -886,6 +929,21 @@ fn optimize<'gc>( stack.pop(); stack.push_any(); } + Op::LShift => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::RShift => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::URShift => { + stack.pop(); + stack.pop(); + stack.push_any(); + } Op::PushDouble { .. } => { stack.push_number(); } @@ -893,15 +951,26 @@ fn optimize<'gc>( stack.push_class_object(activation.avm2().classes().string); } Op::NewArray { num_args } => { - for _ in 0..num_args { + for _ in 0..*num_args { stack.pop(); } stack.push_class_object(activation.avm2().classes().array); } + Op::NewObject { num_args } => { + for _ in 0..*num_args { + stack.pop(); + stack.pop(); + } + + stack.push_class_object(activation.avm2().classes().object); + } Op::NewFunction { .. } => { stack.push_class_object(activation.avm2().classes().function); } + Op::NewClass { .. } => { + stack.push_class_object(activation.avm2().classes().class); + } Op::IsType { .. } => { stack.pop(); stack.push_boolean(); @@ -920,6 +989,11 @@ fn optimize<'gc>( stack.push_any(); } + Op::AsTypeLate => { + stack.pop(); + stack.pop(); + stack.push_any(); + } Op::AsType { type_name: name_index, } => { @@ -1002,7 +1076,20 @@ fn optimize<'gc>( } } Op::PushScope => { - stack.pop(); + let stack_value = stack.pop(); + if let Some(value) = stack_value { + scope_stack.push(value); + } + } + Op::PushWith => { + // TODO: Some way to mark scopes as with-scope vs normal-scope? + let stack_value = stack.pop(); + if let Some(value) = stack_value { + scope_stack.push(value); + } + } + Op::PopScope => { + scope_stack.pop(); } Op::Pop => { stack.pop(); @@ -1014,6 +1101,11 @@ fn optimize<'gc>( stack.push(stack_value); } } + Op::Kill { index } => { + if (*index as usize) < local_types.len() { + local_types.set_any(*index as usize); + } + } Op::SetLocal { index } => { let stack_value = stack.pop(); if (*index as usize) < local_types.len() { @@ -1110,6 +1202,12 @@ fn optimize<'gc>( stack.push_any(); } } + Op::GetSlot { .. } => { + stack.pop(); + + // Avoid handling type for now + stack.push_any(); + } Op::InitProperty { index: name_index } => { stack.pop(); let stack_value = stack.pop(); @@ -1167,7 +1265,6 @@ fn optimize<'gc>( stack.pop(); } - // Then receiver. stack.pop(); // Avoid checking return value for now @@ -1236,6 +1333,25 @@ fn optimize<'gc>( // Avoid handling for now stack.clear(); } + Op::Call { num_args } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + stack.pop(); + + // Avoid checking return value for now + stack.push_any(); + } + Op::GetGlobalScope => { + // Avoid handling for now + stack.push_any(); + } + Op::NewActivation => { + // Avoid handling for now + stack.push_any(); + } Op::Nop => {} Op::DebugFile { .. } => {} Op::DebugLine { .. } => {} @@ -1266,8 +1382,14 @@ fn optimize<'gc>( stack.pop(); stack.push_int(); } + Op::ReturnVoid | Op::ReturnValue | Op::Jump { .. } | Op::LookupSwitch(_) => { + stack.clear(); + scope_stack.clear(); + local_types = initial_local_types.clone(); + } _ => { stack.clear(); + scope_stack.clear(); local_types = initial_local_types.clone(); } } From 51774a51b15b572a7e3ff8c6f147fe09905ac83a Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sun, 4 Feb 2024 08:58:45 -0800 Subject: [PATCH 07/13] avm2: Also optimize `CallProperty` to `CallMethod` when possible --- core/src/avm2/verify.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index 1c744af9d38c..e7e81280cfd8 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -1314,13 +1314,27 @@ fn optimize<'gc>( } // Then receiver. - stack.pop(); + let stack_value = stack.pop(); let multiname = method .translation_unit() .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); if let Ok(multiname) = multiname { - if multiname.has_lazy_component() { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Method { disp_id }) => { + *op = Op::CallMethod { + num_args: *num_args, + index: Index::new(disp_id), + }; + } + _ => {} + } + } + } + } else { // Avoid handling lazy for now stack.clear(); } From 13b592e809d3006b5acfad8ef01d3a7864fb87ee Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Tue, 6 Feb 2024 17:56:56 -0800 Subject: [PATCH 08/13] avm2: Add `push_raw` for faster stack pushes --- core/src/avm2.rs | 10 +++ core/src/avm2/activation.rs | 124 +++++++++++++++++++----------------- 2 files changed, 75 insertions(+), 59 deletions(-) diff --git a/core/src/avm2.rs b/core/src/avm2.rs index 0847c9e80035..ed5bf631556b 100644 --- a/core/src/avm2.rs +++ b/core/src/avm2.rs @@ -622,6 +622,16 @@ impl<'gc> Avm2<'gc> { self.stack.push(value); } + /// Push a value onto the operand stack. + /// This is like `push`, but does not handle `PrimitiveObject` + /// and does not check for stack overflows. + #[inline(always)] + fn push_raw(&mut self, value: impl Into>) { + let value = value.into(); + avm_debug!(self, "Stack push {}: {value:?}", self.stack.len()); + self.stack.push(value); + } + /// Retrieve the top-most value on the operand stack. #[allow(clippy::let_and_return)] #[inline(always)] diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index 21d1ddad222e..fe34026758fc 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -647,6 +647,12 @@ impl<'a, 'gc> Activation<'a, 'gc> { self.avm2().push(value.into(), stack_depth, max_stack_size) } + /// Pushes a value onto the operand stack, without running some checks. + #[inline] + pub fn push_raw(&mut self, value: impl Into>) { + self.avm2().push_raw(value) + } + /// Pops a value off the operand stack. #[inline] #[must_use] @@ -1500,7 +1506,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let object = self.pop_stack(); let object = object.coerce_to_object_or_typeerror(self, Some(&multiname))?; let did_delete = object.delete_property(self, &multiname)?; - self.push_stack(did_delete); + self.push_raw(did_delete); return Ok(FrameControl::Continue); } @@ -1522,7 +1528,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { self.context.gc_context, ); - self.push_stack(true); + self.push_raw(true); return Ok(FrameControl::Continue); } } @@ -1534,7 +1540,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let object = object.coerce_to_object_or_typeerror(self, Some(&multiname))?; let did_delete = object.delete_property(self, &multiname)?; - self.push_stack(did_delete); + self.push_raw(did_delete); Ok(FrameControl::Continue) } @@ -1583,7 +1589,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { if let Some(dictionary) = obj.as_dictionary_object() { if !name_value.is_primitive() { let obj_key = name_value.as_object().unwrap(); - self.push_stack(dictionary.has_property_by_object(obj_key)); + self.push_raw(dictionary.has_property_by_object(obj_key)); return Ok(FrameControl::Continue); } @@ -1593,7 +1599,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let multiname = Multiname::new(self.avm2().find_public_namespace(), name); let has_prop = obj.has_property_via_in(self, &multiname)?; - self.push_stack(has_prop); + self.push_raw(has_prop); Ok(FrameControl::Continue) } @@ -1912,7 +1918,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let new_class = ClassObject::from_class(self, class_entry, base_class)?; - self.push_stack(new_class); + self.push_raw(new_class); Ok(FrameControl::Continue) } @@ -1944,7 +1950,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_coerce_b(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_boolean(); - self.push_stack(value); + self.push_raw(value); Ok(FrameControl::Continue) } @@ -1952,7 +1958,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_coerce_d(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_number(self)?; - self.push_stack(value); + self.push_raw(value); Ok(FrameControl::Continue) } @@ -1960,7 +1966,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_coerce_i(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value); + self.push_raw(value); Ok(FrameControl::Continue) } @@ -1986,7 +1992,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { _ => value.coerce_to_string(self)?.into(), }; - self.push_stack(coerced); + self.push_raw(coerced); Ok(FrameControl::Continue) } @@ -1994,7 +2000,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_coerce_u(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_u32(self)?; - self.push_stack(value); + self.push_raw(value); Ok(FrameControl::Continue) } @@ -2012,7 +2018,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_convert_s(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_string(self)?; - self.push_stack(value); + self.push_raw(value); Ok(FrameControl::Continue) } @@ -2099,7 +2105,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_i32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1.wrapping_add(value2)); + self.push_raw(value1.wrapping_add(value2)); Ok(FrameControl::Continue) } @@ -2108,7 +2114,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_i32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1 & value2); + self.push_raw(value1 & value2); Ok(FrameControl::Continue) } @@ -2116,7 +2122,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_bitnot(&mut self) -> Result, Error<'gc>> { let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(!value1); + self.push_raw(!value1); Ok(FrameControl::Continue) } @@ -2125,7 +2131,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_i32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1 | value2); + self.push_raw(value1 | value2); Ok(FrameControl::Continue) } @@ -2134,7 +2140,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_i32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1 ^ value2); + self.push_raw(value1 ^ value2); Ok(FrameControl::Continue) } @@ -2158,7 +2164,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_decrement(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_number(self)?; - self.push_stack(value - 1.0); + self.push_raw(value - 1.0); Ok(FrameControl::Continue) } @@ -2166,7 +2172,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_decrement_i(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value.wrapping_sub(1)); + self.push_raw(value.wrapping_sub(1)); Ok(FrameControl::Continue) } @@ -2175,7 +2181,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_number(self)?; let value1 = self.pop_stack().coerce_to_number(self)?; - self.push_stack(value1 / value2); + self.push_raw(value1 / value2); Ok(FrameControl::Continue) } @@ -2199,7 +2205,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_increment(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_number(self)?; - self.push_stack(value + 1.0); + self.push_raw(value + 1.0); Ok(FrameControl::Continue) } @@ -2207,7 +2213,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_increment_i(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value.wrapping_add(1)); + self.push_raw(value.wrapping_add(1)); Ok(FrameControl::Continue) } @@ -2216,7 +2222,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_u32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1 << (value2 & 0x1F)); + self.push_raw(value1 << (value2 & 0x1F)); Ok(FrameControl::Continue) } @@ -2225,7 +2231,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_number(self)?; let value1 = self.pop_stack().coerce_to_number(self)?; - self.push_stack(value1 % value2); + self.push_raw(value1 % value2); Ok(FrameControl::Continue) } @@ -2234,7 +2240,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_number(self)?; let value1 = self.pop_stack().coerce_to_number(self)?; - self.push_stack(value1 * value2); + self.push_raw(value1 * value2); Ok(FrameControl::Continue) } @@ -2243,7 +2249,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_i32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1.wrapping_mul(value2)); + self.push_raw(value1.wrapping_mul(value2)); Ok(FrameControl::Continue) } @@ -2251,7 +2257,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_negate(&mut self) -> Result, Error<'gc>> { let value1 = self.pop_stack().coerce_to_number(self)?; - self.push_stack(-value1); + self.push_raw(-value1); Ok(FrameControl::Continue) } @@ -2259,7 +2265,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_negate_i(&mut self) -> Result, Error<'gc>> { let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1.wrapping_neg()); + self.push_raw(value1.wrapping_neg()); Ok(FrameControl::Continue) } @@ -2268,7 +2274,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_u32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1 >> (value2 & 0x1F)); + self.push_raw(value1 >> (value2 & 0x1F)); Ok(FrameControl::Continue) } @@ -2297,7 +2303,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_i32(self)?; let value1 = self.pop_stack().coerce_to_i32(self)?; - self.push_stack(value1.wrapping_sub(value2)); + self.push_raw(value1.wrapping_sub(value2)); Ok(FrameControl::Continue) } @@ -2316,7 +2322,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value2 = self.pop_stack().coerce_to_u32(self)?; let value1 = self.pop_stack().coerce_to_u32(self)?; - self.push_stack(value1 >> (value2 & 0x1F)); + self.push_raw(value1 >> (value2 & 0x1F)); Ok(FrameControl::Continue) } @@ -2482,7 +2488,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_strict_equals(&mut self) -> Result, Error<'gc>> { let value2 = self.pop_stack(); let value1 = self.pop_stack(); - self.push_stack(value1.strict_eq(&value2)); + self.push_raw(value1.strict_eq(&value2)); Ok(FrameControl::Continue) } @@ -2493,7 +2499,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let result = value1.abstract_eq(&value2, self)?; - self.push_stack(result); + self.push_raw(result); Ok(FrameControl::Continue) } @@ -2504,7 +2510,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let result = !value1.abstract_lt(&value2, self)?.unwrap_or(true); - self.push_stack(result); + self.push_raw(result); Ok(FrameControl::Continue) } @@ -2515,7 +2521,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let result = value2.abstract_lt(&value1, self)?.unwrap_or(false); - self.push_stack(result); + self.push_raw(result); Ok(FrameControl::Continue) } @@ -2526,7 +2532,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let result = !value2.abstract_lt(&value1, self)?.unwrap_or(true); - self.push_stack(result); + self.push_raw(result); Ok(FrameControl::Continue) } @@ -2537,7 +2543,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let result = value1.abstract_lt(&value2, self)?.unwrap_or(false); - self.push_stack(result); + self.push_raw(result); Ok(FrameControl::Continue) } @@ -2545,7 +2551,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn op_not(&mut self) -> Result, Error<'gc>> { let value = self.pop_stack().coerce_to_boolean(); - self.push_stack(!value); + self.push_raw(!value); Ok(FrameControl::Continue) } @@ -2555,13 +2561,13 @@ impl<'a, 'gc> Activation<'a, 'gc> { let object = self.pop_stack(); if matches!(object, Value::Undefined | Value::Null) { - self.push_stack(0.0); + self.push_raw(0.0); } else { let object = object.coerce_to_object(self)?; if let Some(next_index) = object.get_next_enumerant(cur_index, self)? { - self.push_stack(next_index); + self.push_raw(next_index); } else { - self.push_stack(0.0); + self.push_raw(0.0); } } @@ -2596,7 +2602,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { cur_index = 0; } - self.push_stack(cur_index != 0); + self.push_raw(cur_index != 0); self.set_local_register(index_register, cur_index); self.set_local_register( object_register, @@ -2639,7 +2645,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let type_object = self.lookup_class_in_domain(&multiname)?; let is_instance_of = value.is_of_type(self, type_object); - self.push_stack(is_instance_of); + self.push_raw(is_instance_of); Ok(FrameControl::Continue) } @@ -2659,7 +2665,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value = self.pop_stack(); let is_instance_of = value.is_of_type(self, type_object.inner_class_definition()); - self.push_stack(is_instance_of); + self.push_raw(is_instance_of); Ok(FrameControl::Continue) } @@ -2677,7 +2683,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { if value.is_of_type(self, class) { self.push_stack(value); } else { - self.push_stack(Value::Null); + self.push_raw(Value::Null); } Ok(FrameControl::Continue) @@ -2701,7 +2707,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { if value.is_of_type(self, class.inner_class_definition()) { self.push_stack(value); } else { - self.push_stack(Value::Null); + self.push_raw(Value::Null); } Ok(FrameControl::Continue) @@ -2734,13 +2740,13 @@ impl<'a, 'gc> Activation<'a, 'gc> { if let Ok(value) = value.coerce_to_object(self) { let is_instance_of = value.is_instance_of(self, type_object)?; - self.push_stack(is_instance_of); + self.push_raw(is_instance_of); } else if matches!(value, Value::Undefined) { // undefined return Err(make_null_or_undefined_error(self, value, None)); } else { // null - self.push_stack(false); + self.push_raw(false); } Ok(FrameControl::Continue) @@ -2783,7 +2789,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { Value::String(_) => "string", }; - self.push_stack(Value::String(type_name.into())); + self.push_raw(Value::String(type_name.into())); Ok(FrameControl::Continue) } @@ -2794,7 +2800,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { // Implementation of `EscapeAttributeValue` from ECMA-357(10.2.1.2) let r = escape_attribute_value(s); - self.push_stack(AvmString::new(self.context.gc_context, r)); + self.push_raw(AvmString::new(self.context.gc_context, r)); Ok(FrameControl::Continue) } @@ -2809,7 +2815,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { x => AvmString::new(self.gc(), escape_element_value(x.coerce_to_string(self)?)), }; - self.push_stack(r); + self.push_raw(r); Ok(FrameControl::Continue) } @@ -2980,7 +2986,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let val = dm.get(address); if let Some(val) = val { - self.push_stack(val); + self.push_raw(val); } else { return Err(make_error_1506(self)); } @@ -3002,7 +3008,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { } let val = dm.read_at(2, address).map_err(|e| e.to_avm(self))?; - self.push_stack(u16::from_le_bytes(val.try_into().unwrap())); + self.push_raw(u16::from_le_bytes(val.try_into().unwrap())); Ok(FrameControl::Continue) } @@ -3021,7 +3027,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { } let val = dm.read_at(4, address).map_err(|e| e.to_avm(self))?; - self.push_stack(i32::from_le_bytes(val.try_into().unwrap())); + self.push_raw(i32::from_le_bytes(val.try_into().unwrap())); Ok(FrameControl::Continue) } @@ -3039,7 +3045,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { } let val = dm.read_at(4, address).map_err(|e| e.to_avm(self))?; - self.push_stack(f32::from_le_bytes(val.try_into().unwrap())); + self.push_raw(f32::from_le_bytes(val.try_into().unwrap())); Ok(FrameControl::Continue) } @@ -3058,7 +3064,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { } let val = dm.read_at(8, address).map_err(|e| e.to_avm(self))?; - self.push_stack(f64::from_le_bytes(val.try_into().unwrap())); + self.push_raw(f64::from_le_bytes(val.try_into().unwrap())); Ok(FrameControl::Continue) } @@ -3068,7 +3074,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let val = val.wrapping_shl(31).wrapping_shr(31); - self.push_stack(Value::Integer(val)); + self.push_raw(Value::Integer(val)); Ok(FrameControl::Continue) } @@ -3079,7 +3085,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let val = (val.wrapping_shl(23).wrapping_shr(23) & 0xFF) as i8 as i32; - self.push_stack(Value::Integer(val)); + self.push_raw(Value::Integer(val)); Ok(FrameControl::Continue) } @@ -3090,7 +3096,7 @@ impl<'a, 'gc> Activation<'a, 'gc> { let val = (val.wrapping_shl(15).wrapping_shr(15) & 0xFFFF) as i16 as i32; - self.push_stack(Value::Integer(val)); + self.push_raw(Value::Integer(val)); Ok(FrameControl::Continue) } From ad85fc2b697d17898ce7f26157eb4d7b44e10de0 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Tue, 6 Feb 2024 18:15:41 -0800 Subject: [PATCH 09/13] avm2: Extract optimizer logic into a different file --- core/src/avm2.rs | 1 + core/src/avm2/optimize.rs | 899 ++++++++++++++++++++++++++++++++++++++ core/src/avm2/verify.rs | 895 +------------------------------------ 3 files changed, 901 insertions(+), 894 deletions(-) create mode 100644 core/src/avm2/optimize.rs diff --git a/core/src/avm2.rs b/core/src/avm2.rs index ed5bf631556b..4856b6b169dc 100644 --- a/core/src/avm2.rs +++ b/core/src/avm2.rs @@ -51,6 +51,7 @@ mod multiname; mod namespace; pub mod object; mod op; +mod optimize; mod parameters; pub mod property; mod property_map; diff --git a/core/src/avm2/optimize.rs b/core/src/avm2/optimize.rs new file mode 100644 index 000000000000..c6ff18b7f8fa --- /dev/null +++ b/core/src/avm2/optimize.rs @@ -0,0 +1,899 @@ +use crate::avm2::activation::Activation; +use crate::avm2::class::Class; +use crate::avm2::method::BytecodeMethod; +use crate::avm2::object::ClassObject; +use crate::avm2::op::Op; +use crate::avm2::property::Property; + +use gc_arena::GcCell; +use std::collections::HashSet; +use swf::avm2::types::Index; + +#[derive(Clone, Copy, Debug)] +enum ValueType<'gc> { + // Either a class, or null. + Class(ClassObject<'gc>), + Int, + Uint, + Number, + Boolean, + Null, + Any, +} + +#[derive(Clone, Debug)] +struct Locals<'gc>(Vec>); + +impl<'gc> Locals<'gc> { + fn new(size: usize) -> Self { + Self(vec![ValueType::Any; size]) + } + + fn set_class_object(&mut self, index: usize, class: ClassObject<'gc>) { + self.0[index] = ValueType::Class(class); + } + + fn set_class(&mut self, index: usize, class: GcCell<'gc, Class<'gc>>) { + // FIXME: Getting the ClassObject this way should be unnecessary + // after the ClassObject refactor + self.0[index] = class + .read() + .class_objects() + .get(0) + .map(|c| ValueType::Class(*c)) + .unwrap_or(ValueType::Any); + } + + fn set_any(&mut self, index: usize) { + self.0[index] = ValueType::Any; + } + + fn set(&mut self, index: usize, value: ValueType<'gc>) { + self.0[index] = value; + } + + fn at(&self, index: usize) -> Option> { + self.0.get(index).copied() + } + + fn len(&self) -> usize { + self.0.len() + } +} + +#[derive(Clone, Debug)] +struct Stack<'gc>(Vec>); + +impl<'gc> Stack<'gc> { + fn new() -> Self { + Self(Vec::new()) + } + + fn push_class_object(&mut self, class: ClassObject<'gc>) { + self.0.push(ValueType::Class(class)); + } + + fn push_class(&mut self, class: GcCell<'gc, Class<'gc>>) { + // FIXME: Getting the ClassObject this way should be unnecessary + // after the ClassObject refactor + self.0.push( + class + .read() + .class_objects() + .get(0) + .map(|c| ValueType::Class(*c)) + .unwrap_or(ValueType::Any), + ); + } + + fn push_int(&mut self) { + self.0.push(ValueType::Int); + } + + fn push_uint(&mut self) { + self.0.push(ValueType::Uint); + } + + fn push_number(&mut self) { + self.0.push(ValueType::Number); + } + + fn push_boolean(&mut self) { + self.0.push(ValueType::Boolean); + } + + fn push_any(&mut self) { + self.0.push(ValueType::Any); + } + + fn push_null(&mut self) { + self.0.push(ValueType::Null); + } + + fn push(&mut self, value: ValueType<'gc>) { + self.0.push(value); + } + + fn pop(&mut self) -> Option> { + self.0.pop() + } + + fn clear(&mut self) { + self.0 = Vec::new(); + } +} + +pub fn optimize<'gc>( + activation: &mut Activation<'_, 'gc>, + method: &BytecodeMethod<'gc>, + code: &mut Vec, + jump_targets: HashSet, +) { + // These make the code less readable + #![allow(clippy::manual_filter)] + #![allow(clippy::single_match)] + + let method_body = method + .body() + .expect("Cannot verify non-native method without body!"); + + // This can probably be done better by recording the receiver in `Activation`, + // but this works since it's guaranteed to be set in `Activation::from_method`. + let this_value = activation.local_register(0); + + let this_class = if let Some(this_class) = activation.subclass_object() { + if this_value.is_of_type(activation, this_class.inner_class_definition()) { + Some(this_class) + } else { + None + } + } else { + None + }; + + // TODO: Store these argument types somewhere on the function so they don't + // have to be re-resolved every function call + let mut argument_types = Vec::new(); + for argument in &method.signature { + let type_name = &argument.param_type_name; + + let argument_type = if !type_name.has_lazy_component() { + activation + .domain() + .get_class(type_name, activation.context.gc_context) + } else { + None + }; + + argument_types.push(argument_type); + } + + // Initial set of local types + let mut initial_local_types = Locals::new(method_body.num_locals as usize); + if let Some(this_class) = this_class { + initial_local_types.set_class_object(0, this_class); + } + + let mut i = 1; + for argument_type in argument_types { + if let Some(argument_type) = argument_type { + initial_local_types.set_class(i, argument_type); + } + i += 1; + } + + // Logic to only allow for type-based optimizations on types that + // we're absolutely sure about- invalidate the local register's + // known type if any other register-modifying opcodes mention them + // anywhere else in the function. + for op in &*code { + match op { + Op::SetLocal { index } + | Op::Kill { index } + | Op::IncLocal { index } + | Op::IncLocalI { index } + | Op::DecLocal { index } + | Op::DecLocalI { index } => { + if (*index as usize) < initial_local_types.len() { + initial_local_types.set_any(*index as usize); + } + } + Op::HasNext2 { + object_register, + index_register, + } => { + if (*object_register as usize) < initial_local_types.len() { + initial_local_types.set_any(*object_register as usize); + } + if (*index_register as usize) < initial_local_types.len() { + initial_local_types.set_any(*index_register as usize); + } + } + _ => {} + } + } + + let mut stack = Stack::new(); + let mut scope_stack = Stack::new(); + let mut local_types = initial_local_types.clone(); + + for (i, op) in code.iter_mut().enumerate() { + if jump_targets.contains(&(i as i32)) { + stack.clear(); + scope_stack.clear(); + local_types = initial_local_types.clone(); + } + + match op { + Op::CoerceB => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Boolean)) { + *op = Op::Nop; + } + stack.push_boolean(); + } + Op::CoerceD => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Number)) { + *op = Op::Nop; + } + stack.push_number(); + } + Op::CoerceI => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Int)) + || matches!(stack_value, Some(ValueType::Uint)) + { + *op = Op::Nop; + } + stack.push_int(); + } + Op::CoerceU => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Uint)) { + *op = Op::Nop; + } + stack.push_uint(); + } + Op::CoerceA => { + stack.pop(); + stack.push_any(); + } + Op::CoerceS => { + let stack_value = stack.pop(); + if matches!(stack_value, Some(ValueType::Null)) { + *op = Op::Nop; + } + stack.push_class_object(activation.avm2().classes().string); + } + Op::Equals + | Op::StrictEquals + | Op::LessEquals + | Op::LessThan + | Op::GreaterThan + | Op::GreaterEquals => { + stack.pop(); + stack.pop(); + stack.push_boolean(); + } + Op::Not => { + stack.pop(); + stack.push_boolean(); + } + Op::PushTrue | Op::PushFalse => { + stack.push_boolean(); + } + Op::PushNull => { + stack.push_null(); + } + Op::PushUndefined => { + stack.push_any(); + } + Op::PushNaN => { + stack.push_number(); + } + Op::PushByte { value } => { + if *value >= 0 { + stack.push_uint(); + } else { + stack.push_int(); + } + } + Op::PushShort { value } => { + if *value >= 0 { + stack.push_uint(); + } else { + stack.push_int(); + } + } + Op::PushInt { value } => { + if *value < -(1 << 28) || *value >= (1 << 28) { + stack.push_number(); + } else if *value >= 0 { + stack.push_uint(); + } else { + stack.push_int(); + } + } + Op::DecrementI => { + // This doesn't give any Number-int guarantees + stack.pop(); + stack.push_any(); + } + Op::IncrementI => { + // This doesn't give any Number-int guarantees + stack.pop(); + stack.push_any(); + } + Op::DecLocalI { index } => { + if (*index as usize) < local_types.len() { + // This doesn't give any Number-int guarantees + local_types.set_any(*index as usize); + } + } + Op::IncLocalI { index } => { + if (*index as usize) < local_types.len() { + // This doesn't give any Number-int guarantees + local_types.set_any(*index as usize); + } + } + Op::Increment => { + stack.pop(); + stack.push_number(); + } + Op::Decrement => { + stack.pop(); + stack.push_number(); + } + Op::Negate => { + stack.pop(); + stack.push_number(); + } + Op::AddI => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::SubtractI => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::MultiplyI => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::Add => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::Subtract => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::Multiply => { + stack.pop(); + stack.pop(); + + // NOTE: In our current implementation, this is guaranteed, + // but it may not be after correctness fixes to match avmplus + stack.push_number(); + } + Op::Divide => { + stack.pop(); + stack.pop(); + + // NOTE: In our current implementation, this is guaranteed, + // but it may not be after correctness fixes to match avmplus + stack.push_number(); + } + Op::Modulo => { + stack.pop(); + stack.pop(); + + // NOTE: In our current implementation, this is guaranteed, + // but it may not be after correctness fixes to match avmplus + stack.push_number(); + } + Op::BitNot => { + stack.pop(); + stack.push_any(); + } + Op::BitAnd => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::BitOr => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::BitXor => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::LShift => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::RShift => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::URShift => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::PushDouble { .. } => { + stack.push_number(); + } + Op::PushString { .. } => { + stack.push_class_object(activation.avm2().classes().string); + } + Op::NewArray { num_args } => { + for _ in 0..*num_args { + stack.pop(); + } + + stack.push_class_object(activation.avm2().classes().array); + } + Op::NewObject { num_args } => { + for _ in 0..*num_args { + stack.pop(); + stack.pop(); + } + + stack.push_class_object(activation.avm2().classes().object); + } + Op::NewFunction { .. } => { + stack.push_class_object(activation.avm2().classes().function); + } + Op::NewClass { .. } => { + stack.push_class_object(activation.avm2().classes().class); + } + Op::IsType { .. } => { + stack.pop(); + stack.push_boolean(); + } + Op::IsTypeLate => { + stack.pop(); + stack.pop(); + stack.push_boolean(); + } + Op::ApplyType { num_types } => { + for _ in 0..*num_types { + stack.pop(); + } + + stack.pop(); + + stack.push_any(); + } + Op::AsTypeLate => { + stack.pop(); + stack.pop(); + stack.push_any(); + } + Op::AsType { + type_name: name_index, + } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + + let resolved_type = if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + activation + .domain() + .get_class(&multiname, activation.context.gc_context) + } else { + None + } + } else { + None + }; + + let stack_value = stack.pop(); + if resolved_type.is_some() && matches!(stack_value, Some(ValueType::Null)) { + *op = Op::Nop; + } + + if let Some(resolved_type) = resolved_type { + stack.push_class(resolved_type); + } else { + stack.push_any(); + } + } + Op::Coerce { index: name_index } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + + let resolved_type = if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + activation + .domain() + .get_class(&multiname, activation.context.gc_context) + } else { + None + } + } else { + None + }; + + let stack_value = stack.pop(); + if let Some(resolved_type) = resolved_type { + if matches!(stack_value, Some(ValueType::Null)) { + // As long as this Coerce isn't coercing to one + // of these special classes, we can remove it. + if !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().int.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().uint.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().number.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().boolean.inner_class_definition(), + ) && !GcCell::ptr_eq( + resolved_type, + activation.avm2().classes().void.inner_class_definition(), + ) { + *op = Op::Nop; + } + } else if let Some(ValueType::Class(class_object)) = stack_value { + if GcCell::ptr_eq(resolved_type, class_object.inner_class_definition()) { + *op = Op::Nop; + } + } + + stack.push_class(resolved_type); + } else { + stack.push_any(); + } + } + Op::PushScope => { + let stack_value = stack.pop(); + if let Some(value) = stack_value { + scope_stack.push(value); + } + } + Op::PushWith => { + // TODO: Some way to mark scopes as with-scope vs normal-scope? + let stack_value = stack.pop(); + if let Some(value) = stack_value { + scope_stack.push(value); + } + } + Op::PopScope => { + scope_stack.pop(); + } + Op::Pop => { + stack.pop(); + } + Op::Dup => { + let stack_value = stack.pop(); + if let Some(stack_value) = stack_value { + stack.push(stack_value); + stack.push(stack_value); + } + } + Op::Kill { index } => { + if (*index as usize) < local_types.len() { + local_types.set_any(*index as usize); + } + } + Op::SetLocal { index } => { + let stack_value = stack.pop(); + if (*index as usize) < local_types.len() { + if let Some(stack_value) = stack_value { + local_types.set(*index as usize, stack_value); + } else { + local_types.set_any(*index as usize); + } + } + } + Op::GetLocal { index } => { + let local_type = local_types.at(*index as usize); + if let Some(local_type) = local_type { + stack.push(local_type); + } else { + stack.push_any(); + } + } + Op::GetLex { .. } => { + stack.push_any(); + } + Op::FindPropStrict { index: name_index } => { + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + stack.push_any(); + } else { + // Avoid handling lazy for now + stack.clear(); + } + } + } + Op::FindProperty { .. } => { + // Avoid handling for now + stack.clear(); + } + Op::GetProperty { index: name_index } => { + let mut stack_push_done = false; + let stack_value = stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) + | Some(Property::ConstSlot { slot_id }) => { + *op = Op::GetSlot { index: slot_id }; + + let mut value_class = + class.instance_vtable().slot_classes() + [slot_id as usize]; + let resolved_value_class = + value_class.get_class(activation); + if let Ok(class) = resolved_value_class { + stack_push_done = true; + + if let Some(class) = class { + stack.push_class(class); + } else { + stack.push_any(); + } + } + + class.instance_vtable().set_slot_class( + activation.context.gc_context, + slot_id as usize, + value_class, + ); + } + Some(Property::Virtual { get: Some(get), .. }) => { + *op = Op::CallMethod { + num_args: 0, + index: Index::new(get), + }; + } + _ => {} + } + } + } + } else { + // Avoid handling lazy for now + stack.clear(); + } + } + + if !stack_push_done { + stack.push_any(); + } + } + Op::GetSlot { .. } => { + stack.pop(); + + // Avoid handling type for now + stack.push_any(); + } + Op::InitProperty { index: name_index } => { + stack.pop(); + let stack_value = stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) + | Some(Property::ConstSlot { slot_id }) => { + *op = Op::SetSlot { index: slot_id }; + } + _ => {} + } + } + } + } else { + // Avoid handling lazy for now + stack.clear(); + } + } + } + Op::SetProperty { index: name_index } => { + stack.pop(); + let stack_value = stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Slot { slot_id }) => { + *op = Op::SetSlot { index: slot_id }; + } + _ => {} + } + } + } + } else { + // Avoid handling lazy for now + stack.clear(); + } + } + } + Op::Construct { num_args } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + stack.pop(); + + // Avoid checking return value for now + stack.push_any(); + } + Op::ConstructSuper { num_args } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + stack.pop(); + } + Op::ConstructProp { + index: name_index, + num_args, + } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if multiname.has_lazy_component() { + // Avoid handling lazy for now + stack.clear(); + } + } + + // Avoid checking return value for now + stack.push_any(); + } + Op::CallProperty { + index: name_index, + num_args, + } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + // Then receiver. + let stack_value = stack.pop(); + + let multiname = method + .translation_unit() + .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); + if let Ok(multiname) = multiname { + if !multiname.has_lazy_component() { + if let Some(ValueType::Class(class)) = stack_value { + if !class.inner_class_definition().read().is_interface() { + match class.instance_vtable().get_trait(&multiname) { + Some(Property::Method { disp_id }) => { + *op = Op::CallMethod { + num_args: *num_args, + index: Index::new(disp_id), + }; + } + _ => {} + } + } + } + } else { + // Avoid handling lazy for now + stack.clear(); + } + } + + // Avoid checking return value for now + stack.push_any(); + } + Op::CallPropVoid { .. } => { + // Avoid handling for now + stack.clear(); + } + Op::Call { num_args } => { + // Arguments + for _ in 0..*num_args { + stack.pop(); + } + + stack.pop(); + + // Avoid checking return value for now + stack.push_any(); + } + Op::GetGlobalScope => { + // Avoid handling for now + stack.push_any(); + } + Op::NewActivation => { + // Avoid handling for now + stack.push_any(); + } + Op::Nop => {} + Op::DebugFile { .. } => {} + Op::DebugLine { .. } => {} + Op::Debug { .. } => {} + Op::IfTrue { .. } | Op::IfFalse { .. } => { + stack.pop(); + } + Op::IfStrictEq { .. } + | Op::IfStrictNe { .. } + | Op::IfEq { .. } + | Op::IfNe { .. } + | Op::IfGe { .. } + | Op::IfGt { .. } + | Op::IfLe { .. } + | Op::IfLt { .. } + | Op::IfNge { .. } + | Op::IfNgt { .. } + | Op::IfNle { .. } + | Op::IfNlt { .. } => { + stack.pop(); + stack.pop(); + } + Op::Si8 => { + stack.pop(); + stack.pop(); + } + Op::Li8 => { + stack.pop(); + stack.push_int(); + } + Op::ReturnVoid | Op::ReturnValue | Op::Jump { .. } | Op::LookupSwitch(_) => { + stack.clear(); + scope_stack.clear(); + local_types = initial_local_types.clone(); + } + _ => { + stack.clear(); + scope_stack.clear(); + local_types = initial_local_types.clone(); + } + } + } +} diff --git a/core/src/avm2/verify.rs b/core/src/avm2/verify.rs index e7e81280cfd8..378832c7b295 100644 --- a/core/src/avm2/verify.rs +++ b/core/src/avm2/verify.rs @@ -1,14 +1,10 @@ -use crate::avm2::class::Class; use crate::avm2::error::{ make_error_1025, make_error_1032, make_error_1054, make_error_1107, verify_error, }; use crate::avm2::method::BytecodeMethod; -use crate::avm2::object::ClassObject; use crate::avm2::op::Op; -use crate::avm2::property::Property; use crate::avm2::script::TranslationUnit; use crate::avm2::{Activation, Error}; -use gc_arena::GcCell; use std::collections::{HashMap, HashSet}; use swf::avm2::read::Reader; use swf::avm2::types::{ @@ -255,7 +251,7 @@ pub fn verify_method<'gc>( verified_code.push(resolved_op); } - optimize( + crate::avm2::optimize::optimize( activation, method, &mut verified_code, @@ -521,895 +517,6 @@ fn verify_code_starting_from<'gc>( Ok(()) } -#[derive(Clone, Copy, Debug)] -enum ValueType<'gc> { - // Either a class, or null. - Class(ClassObject<'gc>), - Int, - Uint, - Number, - Boolean, - Null, - Any, -} - -#[derive(Clone, Debug)] -struct Locals<'gc>(Vec>); - -impl<'gc> Locals<'gc> { - fn new(size: usize) -> Self { - Self(vec![ValueType::Any; size]) - } - - fn set_class_object(&mut self, index: usize, class: ClassObject<'gc>) { - self.0[index] = ValueType::Class(class); - } - - fn set_class(&mut self, index: usize, class: GcCell<'gc, Class<'gc>>) { - // FIXME: Getting the ClassObject this way should be unnecessary - // after the ClassObject refactor - self.0[index] = class - .read() - .class_objects() - .get(0) - .map(|c| ValueType::Class(*c)) - .unwrap_or(ValueType::Any); - } - - fn set_any(&mut self, index: usize) { - self.0[index] = ValueType::Any; - } - - fn set(&mut self, index: usize, value: ValueType<'gc>) { - self.0[index] = value; - } - - fn at(&self, index: usize) -> Option> { - self.0.get(index).copied() - } - - fn len(&self) -> usize { - self.0.len() - } -} - -#[derive(Clone, Debug)] -struct Stack<'gc>(Vec>); - -impl<'gc> Stack<'gc> { - fn new() -> Self { - Self(Vec::new()) - } - - fn push_class_object(&mut self, class: ClassObject<'gc>) { - self.0.push(ValueType::Class(class)); - } - - fn push_class(&mut self, class: GcCell<'gc, Class<'gc>>) { - // FIXME: Getting the ClassObject this way should be unnecessary - // after the ClassObject refactor - self.0.push( - class - .read() - .class_objects() - .get(0) - .map(|c| ValueType::Class(*c)) - .unwrap_or(ValueType::Any), - ); - } - - fn push_int(&mut self) { - self.0.push(ValueType::Int); - } - - fn push_uint(&mut self) { - self.0.push(ValueType::Uint); - } - - fn push_number(&mut self) { - self.0.push(ValueType::Number); - } - - fn push_boolean(&mut self) { - self.0.push(ValueType::Boolean); - } - - fn push_any(&mut self) { - self.0.push(ValueType::Any); - } - - fn push_null(&mut self) { - self.0.push(ValueType::Null); - } - - fn push(&mut self, value: ValueType<'gc>) { - self.0.push(value); - } - - fn pop(&mut self) -> Option> { - self.0.pop() - } - - fn clear(&mut self) { - self.0 = Vec::new(); - } -} - -fn optimize<'gc>( - activation: &mut Activation<'_, 'gc>, - method: &BytecodeMethod<'gc>, - code: &mut Vec, - jump_targets: HashSet, -) { - // These make the code less readable - #![allow(clippy::manual_filter)] - #![allow(clippy::single_match)] - - let method_body = method - .body() - .expect("Cannot verify non-native method without body!"); - - // This can probably be done better by recording the receiver in `Activation`, - // but this works since it's guaranteed to be set in `Activation::from_method`. - let this_value = activation.local_register(0); - - let this_class = if let Some(this_class) = activation.subclass_object() { - if this_value.is_of_type(activation, this_class.inner_class_definition()) { - Some(this_class) - } else { - None - } - } else { - None - }; - - // TODO: Store these argument types somewhere on the function so they don't - // have to be re-resolved every function call - let mut argument_types = Vec::new(); - for argument in &method.signature { - let type_name = &argument.param_type_name; - - let argument_type = if !type_name.has_lazy_component() { - activation - .domain() - .get_class(type_name, activation.context.gc_context) - } else { - None - }; - - argument_types.push(argument_type); - } - - // Initial set of local types - let mut initial_local_types = Locals::new(method_body.num_locals as usize); - if let Some(this_class) = this_class { - initial_local_types.set_class_object(0, this_class); - } - - let mut i = 1; - for argument_type in argument_types { - if let Some(argument_type) = argument_type { - initial_local_types.set_class(i, argument_type); - } - i += 1; - } - - // Logic to only allow for type-based optimizations on types that - // we're absolutely sure about- invalidate the local register's - // known type if any other register-modifying opcodes mention them - // anywhere else in the function. - for op in &*code { - match op { - Op::SetLocal { index } - | Op::Kill { index } - | Op::IncLocal { index } - | Op::IncLocalI { index } - | Op::DecLocal { index } - | Op::DecLocalI { index } => { - if (*index as usize) < initial_local_types.len() { - initial_local_types.set_any(*index as usize); - } - } - Op::HasNext2 { - object_register, - index_register, - } => { - if (*object_register as usize) < initial_local_types.len() { - initial_local_types.set_any(*object_register as usize); - } - if (*index_register as usize) < initial_local_types.len() { - initial_local_types.set_any(*index_register as usize); - } - } - _ => {} - } - } - - let mut stack = Stack::new(); - let mut scope_stack = Stack::new(); - let mut local_types = initial_local_types.clone(); - - for (i, op) in code.iter_mut().enumerate() { - if jump_targets.contains(&(i as i32)) { - stack.clear(); - scope_stack.clear(); - local_types = initial_local_types.clone(); - } - - match op { - Op::CoerceB => { - let stack_value = stack.pop(); - if matches!(stack_value, Some(ValueType::Boolean)) { - *op = Op::Nop; - } - stack.push_boolean(); - } - Op::CoerceD => { - let stack_value = stack.pop(); - if matches!(stack_value, Some(ValueType::Number)) { - *op = Op::Nop; - } - stack.push_number(); - } - Op::CoerceI => { - let stack_value = stack.pop(); - if matches!(stack_value, Some(ValueType::Int)) - || matches!(stack_value, Some(ValueType::Uint)) - { - *op = Op::Nop; - } - stack.push_int(); - } - Op::CoerceU => { - let stack_value = stack.pop(); - if matches!(stack_value, Some(ValueType::Uint)) { - *op = Op::Nop; - } - stack.push_uint(); - } - Op::CoerceA => { - stack.pop(); - stack.push_any(); - } - Op::CoerceS => { - let stack_value = stack.pop(); - if matches!(stack_value, Some(ValueType::Null)) { - *op = Op::Nop; - } - stack.push_class_object(activation.avm2().classes().string); - } - Op::Equals - | Op::StrictEquals - | Op::LessEquals - | Op::LessThan - | Op::GreaterThan - | Op::GreaterEquals => { - stack.pop(); - stack.pop(); - stack.push_boolean(); - } - Op::Not => { - stack.pop(); - stack.push_boolean(); - } - Op::PushTrue | Op::PushFalse => { - stack.push_boolean(); - } - Op::PushNull => { - stack.push_null(); - } - Op::PushUndefined => { - stack.push_any(); - } - Op::PushNaN => { - stack.push_number(); - } - Op::PushByte { value } => { - if *value >= 0 { - stack.push_uint(); - } else { - stack.push_int(); - } - } - Op::PushShort { value } => { - if *value >= 0 { - stack.push_uint(); - } else { - stack.push_int(); - } - } - Op::PushInt { value } => { - if *value < -(1 << 28) || *value >= (1 << 28) { - stack.push_number(); - } else if *value >= 0 { - stack.push_uint(); - } else { - stack.push_int(); - } - } - Op::DecrementI => { - // This doesn't give any Number-int guarantees - stack.pop(); - stack.push_any(); - } - Op::IncrementI => { - // This doesn't give any Number-int guarantees - stack.pop(); - stack.push_any(); - } - Op::DecLocalI { index } => { - if (*index as usize) < local_types.len() { - // This doesn't give any Number-int guarantees - local_types.set_any(*index as usize); - } - } - Op::IncLocalI { index } => { - if (*index as usize) < local_types.len() { - // This doesn't give any Number-int guarantees - local_types.set_any(*index as usize); - } - } - Op::Increment => { - stack.pop(); - stack.push_number(); - } - Op::Decrement => { - stack.pop(); - stack.push_number(); - } - Op::Negate => { - stack.pop(); - stack.push_number(); - } - Op::AddI => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::SubtractI => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::MultiplyI => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::Add => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::Subtract => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::Multiply => { - stack.pop(); - stack.pop(); - - // NOTE: In our current implementation, this is guaranteed, - // but it may not be after correctness fixes to match avmplus - stack.push_number(); - } - Op::Divide => { - stack.pop(); - stack.pop(); - - // NOTE: In our current implementation, this is guaranteed, - // but it may not be after correctness fixes to match avmplus - stack.push_number(); - } - Op::Modulo => { - stack.pop(); - stack.pop(); - - // NOTE: In our current implementation, this is guaranteed, - // but it may not be after correctness fixes to match avmplus - stack.push_number(); - } - Op::BitNot => { - stack.pop(); - stack.push_any(); - } - Op::BitAnd => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::BitOr => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::BitXor => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::LShift => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::RShift => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::URShift => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::PushDouble { .. } => { - stack.push_number(); - } - Op::PushString { .. } => { - stack.push_class_object(activation.avm2().classes().string); - } - Op::NewArray { num_args } => { - for _ in 0..*num_args { - stack.pop(); - } - - stack.push_class_object(activation.avm2().classes().array); - } - Op::NewObject { num_args } => { - for _ in 0..*num_args { - stack.pop(); - stack.pop(); - } - - stack.push_class_object(activation.avm2().classes().object); - } - Op::NewFunction { .. } => { - stack.push_class_object(activation.avm2().classes().function); - } - Op::NewClass { .. } => { - stack.push_class_object(activation.avm2().classes().class); - } - Op::IsType { .. } => { - stack.pop(); - stack.push_boolean(); - } - Op::IsTypeLate => { - stack.pop(); - stack.pop(); - stack.push_boolean(); - } - Op::ApplyType { num_types } => { - for _ in 0..*num_types { - stack.pop(); - } - - stack.pop(); - - stack.push_any(); - } - Op::AsTypeLate => { - stack.pop(); - stack.pop(); - stack.push_any(); - } - Op::AsType { - type_name: name_index, - } => { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - - let resolved_type = if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - activation - .domain() - .get_class(&multiname, activation.context.gc_context) - } else { - None - } - } else { - None - }; - - let stack_value = stack.pop(); - if resolved_type.is_some() && matches!(stack_value, Some(ValueType::Null)) { - *op = Op::Nop; - } - - if let Some(resolved_type) = resolved_type { - stack.push_class(resolved_type); - } else { - stack.push_any(); - } - } - Op::Coerce { index: name_index } => { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - - let resolved_type = if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - activation - .domain() - .get_class(&multiname, activation.context.gc_context) - } else { - None - } - } else { - None - }; - - let stack_value = stack.pop(); - if let Some(resolved_type) = resolved_type { - if matches!(stack_value, Some(ValueType::Null)) { - // As long as this Coerce isn't coercing to one - // of these special classes, we can remove it. - if !GcCell::ptr_eq( - resolved_type, - activation.avm2().classes().int.inner_class_definition(), - ) && !GcCell::ptr_eq( - resolved_type, - activation.avm2().classes().uint.inner_class_definition(), - ) && !GcCell::ptr_eq( - resolved_type, - activation.avm2().classes().number.inner_class_definition(), - ) && !GcCell::ptr_eq( - resolved_type, - activation.avm2().classes().boolean.inner_class_definition(), - ) && !GcCell::ptr_eq( - resolved_type, - activation.avm2().classes().void.inner_class_definition(), - ) { - *op = Op::Nop; - } - } else if let Some(ValueType::Class(class_object)) = stack_value { - if GcCell::ptr_eq(resolved_type, class_object.inner_class_definition()) { - *op = Op::Nop; - } - } - - stack.push_class(resolved_type); - } else { - stack.push_any(); - } - } - Op::PushScope => { - let stack_value = stack.pop(); - if let Some(value) = stack_value { - scope_stack.push(value); - } - } - Op::PushWith => { - // TODO: Some way to mark scopes as with-scope vs normal-scope? - let stack_value = stack.pop(); - if let Some(value) = stack_value { - scope_stack.push(value); - } - } - Op::PopScope => { - scope_stack.pop(); - } - Op::Pop => { - stack.pop(); - } - Op::Dup => { - let stack_value = stack.pop(); - if let Some(stack_value) = stack_value { - stack.push(stack_value); - stack.push(stack_value); - } - } - Op::Kill { index } => { - if (*index as usize) < local_types.len() { - local_types.set_any(*index as usize); - } - } - Op::SetLocal { index } => { - let stack_value = stack.pop(); - if (*index as usize) < local_types.len() { - if let Some(stack_value) = stack_value { - local_types.set(*index as usize, stack_value); - } else { - local_types.set_any(*index as usize); - } - } - } - Op::GetLocal { index } => { - let local_type = local_types.at(*index as usize); - if let Some(local_type) = local_type { - stack.push(local_type); - } else { - stack.push_any(); - } - } - Op::GetLex { .. } => { - stack.push_any(); - } - Op::FindPropStrict { index: name_index } => { - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - stack.push_any(); - } else { - // Avoid handling lazy for now - stack.clear(); - } - } - } - Op::FindProperty { .. } => { - // Avoid handling for now - stack.clear(); - } - Op::GetProperty { index: name_index } => { - let mut stack_push_done = false; - let stack_value = stack.pop(); - - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - if let Some(ValueType::Class(class)) = stack_value { - if !class.inner_class_definition().read().is_interface() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) - | Some(Property::ConstSlot { slot_id }) => { - *op = Op::GetSlot { index: slot_id }; - - let mut value_class = - class.instance_vtable().slot_classes() - [slot_id as usize]; - let resolved_value_class = - value_class.get_class(activation); - if let Ok(class) = resolved_value_class { - stack_push_done = true; - - if let Some(class) = class { - stack.push_class(class); - } else { - stack.push_any(); - } - } - - class.instance_vtable().set_slot_class( - activation.context.gc_context, - slot_id as usize, - value_class, - ); - } - Some(Property::Virtual { get: Some(get), .. }) => { - *op = Op::CallMethod { - num_args: 0, - index: Index::new(get), - }; - } - _ => {} - } - } - } - } else { - // Avoid handling lazy for now - stack.clear(); - } - } - - if !stack_push_done { - stack.push_any(); - } - } - Op::GetSlot { .. } => { - stack.pop(); - - // Avoid handling type for now - stack.push_any(); - } - Op::InitProperty { index: name_index } => { - stack.pop(); - let stack_value = stack.pop(); - - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - if let Some(ValueType::Class(class)) = stack_value { - if !class.inner_class_definition().read().is_interface() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) - | Some(Property::ConstSlot { slot_id }) => { - *op = Op::SetSlot { index: slot_id }; - } - _ => {} - } - } - } - } else { - // Avoid handling lazy for now - stack.clear(); - } - } - } - Op::SetProperty { index: name_index } => { - stack.pop(); - let stack_value = stack.pop(); - - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - if let Some(ValueType::Class(class)) = stack_value { - if !class.inner_class_definition().read().is_interface() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Slot { slot_id }) => { - *op = Op::SetSlot { index: slot_id }; - } - _ => {} - } - } - } - } else { - // Avoid handling lazy for now - stack.clear(); - } - } - } - Op::Construct { num_args } => { - // Arguments - for _ in 0..*num_args { - stack.pop(); - } - - stack.pop(); - - // Avoid checking return value for now - stack.push_any(); - } - Op::ConstructSuper { num_args } => { - // Arguments - for _ in 0..*num_args { - stack.pop(); - } - - // Then receiver. - stack.pop(); - } - Op::ConstructProp { - index: name_index, - num_args, - } => { - // Arguments - for _ in 0..*num_args { - stack.pop(); - } - - // Then receiver. - stack.pop(); - - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if multiname.has_lazy_component() { - // Avoid handling lazy for now - stack.clear(); - } - } - - // Avoid checking return value for now - stack.push_any(); - } - Op::CallProperty { - index: name_index, - num_args, - } => { - // Arguments - for _ in 0..*num_args { - stack.pop(); - } - - // Then receiver. - let stack_value = stack.pop(); - - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if !multiname.has_lazy_component() { - if let Some(ValueType::Class(class)) = stack_value { - if !class.inner_class_definition().read().is_interface() { - match class.instance_vtable().get_trait(&multiname) { - Some(Property::Method { disp_id }) => { - *op = Op::CallMethod { - num_args: *num_args, - index: Index::new(disp_id), - }; - } - _ => {} - } - } - } - } else { - // Avoid handling lazy for now - stack.clear(); - } - } - - // Avoid checking return value for now - stack.push_any(); - } - Op::CallPropVoid { .. } => { - // Avoid handling for now - stack.clear(); - } - Op::Call { num_args } => { - // Arguments - for _ in 0..*num_args { - stack.pop(); - } - - stack.pop(); - - // Avoid checking return value for now - stack.push_any(); - } - Op::GetGlobalScope => { - // Avoid handling for now - stack.push_any(); - } - Op::NewActivation => { - // Avoid handling for now - stack.push_any(); - } - Op::Nop => {} - Op::DebugFile { .. } => {} - Op::DebugLine { .. } => {} - Op::Debug { .. } => {} - Op::IfTrue { .. } | Op::IfFalse { .. } => { - stack.pop(); - } - Op::IfStrictEq { .. } - | Op::IfStrictNe { .. } - | Op::IfEq { .. } - | Op::IfNe { .. } - | Op::IfGe { .. } - | Op::IfGt { .. } - | Op::IfLe { .. } - | Op::IfLt { .. } - | Op::IfNge { .. } - | Op::IfNgt { .. } - | Op::IfNle { .. } - | Op::IfNlt { .. } => { - stack.pop(); - stack.pop(); - } - Op::Si8 => { - stack.pop(); - stack.pop(); - } - Op::Li8 => { - stack.pop(); - stack.push_int(); - } - Op::ReturnVoid | Op::ReturnValue | Op::Jump { .. } | Op::LookupSwitch(_) => { - stack.clear(); - scope_stack.clear(); - local_types = initial_local_types.clone(); - } - _ => { - stack.clear(); - scope_stack.clear(); - local_types = initial_local_types.clone(); - } - } - } -} - fn ops_can_throw_error(ops: &[AbcOp]) -> bool { for op in ops { match op { From 916d0c2cecbfc199e724a3cb2df850e7ab4de39e Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 19 Feb 2024 12:39:55 -0800 Subject: [PATCH 10/13] avm2: Don't use a `Class`'s first `ClassObject` for its VTable if more than one `ClassObject` had been constructed for the class --- core/src/avm2/class.rs | 8 ++++++++ core/src/avm2/optimize.rs | 10 ++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/core/src/avm2/class.rs b/core/src/avm2/class.rs index eeb4c2da15dd..358c2b5f02ff 100644 --- a/core/src/avm2/class.rs +++ b/core/src/avm2/class.rs @@ -295,6 +295,14 @@ impl<'gc> Class<'gc> { &self.class_objects } + pub fn class_object(&self) -> Option> { + if self.class_objects.len() == 1 { + Some(self.class_objects[0]) + } else { + None + } + } + /// Construct a class from a `TranslationUnit` and its class index. /// /// The returned class will be allocated, but no traits will be loaded. The diff --git a/core/src/avm2/optimize.rs b/core/src/avm2/optimize.rs index c6ff18b7f8fa..e4a1b34866b1 100644 --- a/core/src/avm2/optimize.rs +++ b/core/src/avm2/optimize.rs @@ -38,9 +38,8 @@ impl<'gc> Locals<'gc> { // after the ClassObject refactor self.0[index] = class .read() - .class_objects() - .get(0) - .map(|c| ValueType::Class(*c)) + .class_object() + .map(ValueType::Class) .unwrap_or(ValueType::Any); } @@ -79,9 +78,8 @@ impl<'gc> Stack<'gc> { self.0.push( class .read() - .class_objects() - .get(0) - .map(|c| ValueType::Class(*c)) + .class_object() + .map(ValueType::Class) .unwrap_or(ValueType::Any), ); } From 9f25a3a611c76941e62db4acca1a2dc955bb7fe1 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Mon, 19 Feb 2024 14:05:28 -0800 Subject: [PATCH 11/13] avm2: Add a `popn` function to `Stack` and use it --- core/src/avm2/optimize.rs | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/core/src/avm2/optimize.rs b/core/src/avm2/optimize.rs index e4a1b34866b1..f0dce0d8e5ad 100644 --- a/core/src/avm2/optimize.rs +++ b/core/src/avm2/optimize.rs @@ -116,6 +116,12 @@ impl<'gc> Stack<'gc> { self.0.pop() } + fn popn(&mut self, count: u32) { + for _ in 0..count { + self.pop(); + } + } + fn clear(&mut self) { self.0 = Vec::new(); } @@ -437,17 +443,12 @@ pub fn optimize<'gc>( stack.push_class_object(activation.avm2().classes().string); } Op::NewArray { num_args } => { - for _ in 0..*num_args { - stack.pop(); - } + stack.popn(*num_args); stack.push_class_object(activation.avm2().classes().array); } Op::NewObject { num_args } => { - for _ in 0..*num_args { - stack.pop(); - stack.pop(); - } + stack.popn(*num_args * 2); stack.push_class_object(activation.avm2().classes().object); } @@ -467,9 +468,7 @@ pub fn optimize<'gc>( stack.push_boolean(); } Op::ApplyType { num_types } => { - for _ in 0..*num_types { - stack.pop(); - } + stack.popn(*num_types); stack.pop(); @@ -747,9 +746,7 @@ pub fn optimize<'gc>( } Op::Construct { num_args } => { // Arguments - for _ in 0..*num_args { - stack.pop(); - } + stack.popn(*num_args); stack.pop(); @@ -758,9 +755,7 @@ pub fn optimize<'gc>( } Op::ConstructSuper { num_args } => { // Arguments - for _ in 0..*num_args { - stack.pop(); - } + stack.popn(*num_args); // Then receiver. stack.pop(); @@ -770,9 +765,7 @@ pub fn optimize<'gc>( num_args, } => { // Arguments - for _ in 0..*num_args { - stack.pop(); - } + stack.popn(*num_args); // Then receiver. stack.pop(); @@ -795,9 +788,7 @@ pub fn optimize<'gc>( num_args, } => { // Arguments - for _ in 0..*num_args { - stack.pop(); - } + stack.popn(*num_args); // Then receiver. let stack_value = stack.pop(); @@ -835,9 +826,7 @@ pub fn optimize<'gc>( } Op::Call { num_args } => { // Arguments - for _ in 0..*num_args { - stack.pop(); - } + stack.popn(*num_args); stack.pop(); From d2971e842bcf1078ded91d17fdeda4cedf8bb731 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 24 Feb 2024 12:41:53 -0800 Subject: [PATCH 12/13] avm2: Handle lazy multinames in optimizer --- core/src/avm2/optimize.rs | 85 +++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/core/src/avm2/optimize.rs b/core/src/avm2/optimize.rs index f0dce0d8e5ad..5f85ab4612cf 100644 --- a/core/src/avm2/optimize.rs +++ b/core/src/avm2/optimize.rs @@ -1,11 +1,12 @@ use crate::avm2::activation::Activation; use crate::avm2::class::Class; use crate::avm2::method::BytecodeMethod; +use crate::avm2::multiname::Multiname; use crate::avm2::object::ClassObject; use crate::avm2::op::Op; use crate::avm2::property::Property; -use gc_arena::GcCell; +use gc_arena::{Gc, GcCell}; use std::collections::HashSet; use swf::avm2::types::Index; @@ -116,6 +117,15 @@ impl<'gc> Stack<'gc> { self.0.pop() } + pub fn pop_for_multiname(&mut self, multiname: Gc<'gc, Multiname<'gc>>) { + if multiname.has_lazy_name() { + self.0.pop(); + } + if multiname.has_lazy_ns() { + self.0.pop(); + } + } + fn popn(&mut self, count: u32) { for _ in 0..count { self.pop(); @@ -218,6 +228,23 @@ pub fn optimize<'gc>( } let mut stack = Stack::new(); + + macro_rules! stack_pop_multiname { + ($index: expr) => {{ + let multiname = method + .translation_unit() + // note: ideally this should be a VerifyError here or earlier + .pool_maybe_uninitialized_multiname(*$index, &mut activation.context); + + if let Ok(multiname) = multiname { + stack.pop_for_multiname(multiname); + Some(multiname) + } else { + None + } + }}; + } + let mut scope_stack = Stack::new(); let mut local_types = initial_local_types.clone(); @@ -632,12 +659,11 @@ pub fn optimize<'gc>( } Op::GetProperty { index: name_index } => { let mut stack_push_done = false; + + let multiname = stack_pop_multiname!(name_index); let stack_value = stack.pop(); - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { + if let Some(multiname) = multiname { if !multiname.has_lazy_component() { if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { @@ -677,10 +703,8 @@ pub fn optimize<'gc>( } } } - } else { - // Avoid handling lazy for now - stack.clear(); } + // `stack_pop_multiname` handled lazy } if !stack_push_done { @@ -695,12 +719,11 @@ pub fn optimize<'gc>( } Op::InitProperty { index: name_index } => { stack.pop(); + + let multiname = stack_pop_multiname!(name_index); let stack_value = stack.pop(); - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { + if let Some(multiname) = multiname { if !multiname.has_lazy_component() { if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { @@ -713,20 +736,17 @@ pub fn optimize<'gc>( } } } - } else { - // Avoid handling lazy for now - stack.clear(); } + // `stack_pop_multiname` handled lazy } } Op::SetProperty { index: name_index } => { stack.pop(); + + let multiname = stack_pop_multiname!(name_index); let stack_value = stack.pop(); - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { + if let Some(multiname) = multiname { if !multiname.has_lazy_component() { if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { @@ -738,10 +758,8 @@ pub fn optimize<'gc>( } } } - } else { - // Avoid handling lazy for now - stack.clear(); } + // `stack_pop_multiname` handled lazy } } Op::Construct { num_args } => { @@ -767,19 +785,11 @@ pub fn optimize<'gc>( // Arguments stack.popn(*num_args); + stack_pop_multiname!(name_index); + // Then receiver. stack.pop(); - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { - if multiname.has_lazy_component() { - // Avoid handling lazy for now - stack.clear(); - } - } - // Avoid checking return value for now stack.push_any(); } @@ -790,13 +800,12 @@ pub fn optimize<'gc>( // Arguments stack.popn(*num_args); + let multiname = stack_pop_multiname!(name_index); + // Then receiver. let stack_value = stack.pop(); - let multiname = method - .translation_unit() - .pool_maybe_uninitialized_multiname(*name_index, &mut activation.context); - if let Ok(multiname) = multiname { + if let Some(multiname) = multiname { if !multiname.has_lazy_component() { if let Some(ValueType::Class(class)) = stack_value { if !class.inner_class_definition().read().is_interface() { @@ -811,10 +820,8 @@ pub fn optimize<'gc>( } } } - } else { - // Avoid handling lazy for now - stack.clear(); } + // `stack_pop_multiname` handled lazy } // Avoid checking return value for now From dce4238c9371574f33436827b7a79b8f20a133aa Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Sat, 24 Feb 2024 17:41:47 -0800 Subject: [PATCH 13/13] avm2: Resolve reviews (no functional changes) --- core/src/avm2/optimize.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/core/src/avm2/optimize.rs b/core/src/avm2/optimize.rs index 5f85ab4612cf..a7c8e2c88b39 100644 --- a/core/src/avm2/optimize.rs +++ b/core/src/avm2/optimize.rs @@ -133,7 +133,7 @@ impl<'gc> Stack<'gc> { } fn clear(&mut self) { - self.0 = Vec::new(); + self.0.clear(); } } @@ -188,12 +188,11 @@ pub fn optimize<'gc>( initial_local_types.set_class_object(0, this_class); } - let mut i = 1; - for argument_type in argument_types { + for (i, argument_type) in argument_types.iter().enumerate() { if let Some(argument_type) = argument_type { - initial_local_types.set_class(i, argument_type); + initial_local_types.set_class(i + 1, *argument_type); + // `i + 1` because the receiver takes up local #0 } - i += 1; } // Logic to only allow for type-based optimizations on types that @@ -272,9 +271,7 @@ pub fn optimize<'gc>( } Op::CoerceI => { let stack_value = stack.pop(); - if matches!(stack_value, Some(ValueType::Int)) - || matches!(stack_value, Some(ValueType::Uint)) - { + if matches!(stack_value, Some(ValueType::Int | ValueType::Uint)) { *op = Op::Nop; } stack.push_int(); @@ -287,6 +284,7 @@ pub fn optimize<'gc>( stack.push_uint(); } Op::CoerceA => { + // This does actually inhibit optimizations in FP stack.pop(); stack.push_any(); } @@ -878,7 +876,12 @@ pub fn optimize<'gc>( stack.pop(); stack.push_int(); } - Op::ReturnVoid | Op::ReturnValue | Op::Jump { .. } | Op::LookupSwitch(_) => { + Op::ReturnVoid + | Op::ReturnValue + | Op::Throw + | Op::Jump { .. } + | Op::LookupSwitch(_) => { + // End of block stack.clear(); scope_stack.clear(); local_types = initial_local_types.clone();