Skip to content

Commit

Permalink
avm2: Misc. optimizer improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
Lord-McSweeney authored and Lord-McSweeney committed Jun 8, 2024
1 parent 39f273b commit bb7a315
Showing 1 changed file with 87 additions and 40 deletions.
127 changes: 87 additions & 40 deletions core/src/avm2/optimize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ use crate::avm2::vtable::VTable;
use gc_arena::Gc;
use std::collections::HashMap;

#[allow(clippy::enum_variant_names)]
#[derive(Clone, Copy, Debug)]
enum NullState {
NotNull,
MaybeNull,
IsNull,
}

#[derive(Clone, Copy)]
struct OptValue<'gc> {
// This corresponds to the compile-time assumptions about the type:
Expand All @@ -33,10 +41,9 @@ struct OptValue<'gc> {
// should only be set if class is numeric.
pub contains_valid_unsigned: bool,

// true if value is guaranteed to be null.
// TODO: FP actually has a separate `null` type just for this, this can be observed in VerifyErrors
// (a separate type would also prevent accidental "null int" values)
pub guaranteed_null: bool,
pub null_state: NullState,
}
impl<'gc> OptValue<'gc> {
pub fn any() -> Self {
Expand All @@ -45,15 +52,15 @@ impl<'gc> OptValue<'gc> {
vtable: None,
contains_valid_integer: false,
contains_valid_unsigned: false,
guaranteed_null: false,
null_state: NullState::MaybeNull,
}
}

pub fn null() -> Self {
Self {
class: None,
vtable: None,
guaranteed_null: true,
null_state: NullState::IsNull,
..Self::any()
}
}
Expand All @@ -75,6 +82,25 @@ impl<'gc> OptValue<'gc> {

self.vtable
}

pub fn is_null(self) -> bool {
matches!(self.null_state, NullState::IsNull)
}

pub fn not_null(self, activation: &mut Activation<'_, 'gc>) -> bool {
if matches!(self.null_state, NullState::NotNull) {
return true;
}

let classes = activation.avm2().classes();

// Primitives are always not-null
self.class == Some(classes.int.inner_class_definition())
|| self.class == Some(classes.uint.inner_class_definition())
|| self.class == Some(classes.number.inner_class_definition())
|| self.class == Some(classes.boolean.inner_class_definition())
|| self.class == Some(classes.void.inner_class_definition())
}
}

impl<'gc> std::fmt::Debug for OptValue<'gc> {
Expand All @@ -83,7 +109,7 @@ impl<'gc> std::fmt::Debug for OptValue<'gc> {
.field("class", &self.class)
.field("contains_valid_integer", &self.contains_valid_integer)
.field("contains_valid_unsigned", &self.contains_valid_unsigned)
.field("guaranteed_null", &self.guaranteed_null)
.field("null_state", &self.null_state)
.finish()
}
}
Expand Down Expand Up @@ -130,6 +156,13 @@ impl<'gc> Stack<'gc> {
self.0.push(OptValue::of_type(class));
}

fn push_class_not_null(&mut self, class: Class<'gc>) {
let mut value = OptValue::of_type(class);
value.null_state = NullState::NotNull;

self.0.push(value);
}

fn push_any(&mut self) {
self.0.push(OptValue::any());
}
Expand Down Expand Up @@ -254,7 +287,7 @@ pub fn optimize<'gc>(
vtable: this_vtable,
contains_valid_integer: false,
contains_valid_unsigned: false,
guaranteed_null: false,
null_state: NullState::NotNull,
};

let argument_types = resolved_parameters
Expand Down Expand Up @@ -401,8 +434,7 @@ pub fn optimize<'gc>(
}
Op::CoerceI => {
let stack_value = stack.pop_or_any();
// TODO: maybe the type check is safe here...?
if stack_value.contains_valid_integer {
if stack_value.class == Some(types.int) || stack_value.contains_valid_integer {
*op = Op::Nop;
}
stack.push_class(types.int);
Expand All @@ -420,19 +452,18 @@ pub fn optimize<'gc>(
}
Op::CoerceS => {
let stack_value = stack.pop_or_any();
if stack_value.guaranteed_null {
if stack_value.is_null() {
*op = Op::Nop;
}
stack.push_class(types.string);
}
Op::ConvertS => {
stack.pop();
stack.push_class(types.string);
stack.push_class_not_null(types.string);
}
Op::CoerceU => {
let stack_value = stack.pop_or_any();
// TODO: maybe the type check is safe here...?
if stack_value.contains_valid_unsigned {
if stack_value.class == Some(types.uint) || stack_value.contains_valid_unsigned {
*op = Op::Nop;
}
stack.push_class(types.uint);
Expand Down Expand Up @@ -495,28 +526,27 @@ pub fn optimize<'gc>(
Op::PushUint { value } => {
let mut new_value = OptValue::of_type(types.uint);
if *value < (1 << 28) {
new_value.contains_valid_integer = true;
new_value.contains_valid_unsigned = true;
}
stack.push(new_value);
}
Op::DecrementI => {
// TODO (same for other I ops): analyze what _exactly_ the type int implies
// and whether we can use Number or (u)int here
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::IncrementI => {
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::DecLocalI { index } => {
local_types.set_any(*index as usize);
local_types.set(*index as usize, OptValue::of_type(types.int));
}
Op::DecLocal { index } => {
local_types.set(*index as usize, OptValue::of_type(types.number));
}
Op::IncLocalI { index } => {
local_types.set_any(*index as usize);
local_types.set(*index as usize, OptValue::of_type(types.int));
}
Op::IncLocal { index } => {
local_types.set(*index as usize, OptValue::of_type(types.number));
Expand All @@ -536,21 +566,21 @@ pub fn optimize<'gc>(
Op::AddI => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::SubtractI => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::MultiplyI => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::NegateI => {
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::Add => {
let value2 = stack.pop_or_any();
Expand All @@ -563,6 +593,10 @@ pub fn optimize<'gc>(
|| value2.class == Some(types.number))
{
stack.push_class(types.number);
} else if (value1.class == Some(types.string) && value1.not_null(activation))
|| (value2.class == Some(types.string) && value2.not_null(activation))
{
stack.push_class_not_null(types.string);
} else {
stack.push_any();
}
Expand All @@ -589,62 +623,62 @@ pub fn optimize<'gc>(
}
Op::BitNot => {
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::BitAnd => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::BitOr => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::BitXor => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::LShift => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::RShift => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::URShift => {
stack.pop();
stack.pop();
stack.push_any();
stack.push_class(types.int);
}
Op::PushDouble { .. } => {
stack.push_class(types.number);
}
Op::PushNamespace { .. } => {
stack.push_class(types.namespace);
stack.push_class_not_null(types.namespace);
}
Op::PushString { .. } => {
stack.push_class(types.string);
stack.push_class_not_null(types.string);
}
Op::NewArray { num_args } => {
stack.popn(*num_args);

stack.push_class(types.array);
stack.push_class_not_null(types.array);
}
Op::NewObject { num_args } => {
stack.popn(*num_args * 2);

stack.push_class(types.object);
stack.push_class_not_null(types.object);
}
Op::NewFunction { .. } => {
stack.push_class(types.function);
stack.push_class_not_null(types.function);
}
Op::NewClass { .. } => {
stack.push_class(types.class);
stack.push_class_not_null(types.class);
}
Op::NewCatch { .. } => {
// Avoid handling for now
Expand All @@ -666,7 +700,7 @@ pub fn optimize<'gc>(
}
Op::TypeOf => {
stack.pop();
stack.push_class(types.string);
stack.push_class_not_null(types.string);
}
Op::ApplyType { num_types } => {
stack.popn(*num_types);
Expand All @@ -687,7 +721,7 @@ pub fn optimize<'gc>(
}
Op::EscXAttr | Op::EscXElem => {
stack.pop();
stack.push_class(types.string);
stack.push_class_not_null(types.string);
}
Op::GetDescendants { multiname } => {
stack.pop_for_multiname(*multiname);
Expand Down Expand Up @@ -723,17 +757,18 @@ pub fn optimize<'gc>(
new_value = stack_value;
}
}
if stack_value.guaranteed_null {
if stack_value.is_null() {
// null always turns into null
*op = Op::Nop;
new_value.null_state = NullState::IsNull;
}
stack.push(new_value);
}
Op::Coerce { class } => {
let stack_value = stack.pop_or_any();
stack.push_class(*class);

if stack_value.guaranteed_null {
if stack_value.is_null() {
// Coercing null to a non-primitive or void is a noop.
if *class != types.int
&& *class != types.uint
Expand Down Expand Up @@ -852,7 +887,7 @@ pub fn optimize<'gc>(

if script.traits_loaded() {
stack_push_done = true;
stack.push_class(script.global_class());
stack.push_class_not_null(script.global_class());
}
}
}
Expand Down Expand Up @@ -1370,11 +1405,23 @@ pub fn optimize<'gc>(
let stack_value = stack.pop_or_any();

if let Some(return_type) = return_type {
let return_type_is_primitive = return_type == types.int
|| return_type == types.uint
|| return_type == types.number
|| return_type == types.boolean
|| return_type == types.void;

if let Some(stack_value_class) = stack_value.class {
if stack_value_class == return_type {
*op = Op::ReturnValueNoCoerce;
}
}

if !return_type_is_primitive {
if stack_value.is_null() {
*op = Op::ReturnValueNoCoerce;
}
}
} else {
// Return type was Any, no coercion will be done anyways
*op = Op::ReturnValueNoCoerce;
Expand Down

0 comments on commit bb7a315

Please sign in to comment.