From 746561afa06819f61c7c1089641ae50e18000dac Mon Sep 17 00:00:00 2001 From: Kamil Jarosz Date: Tue, 14 Oct 2025 19:26:54 +0200 Subject: [PATCH] avm2: Allow any i32 in Value::Integer This is a performance optimization, as it gets rid of range checks when operating on values. Since we already implement value normalization, we don't have to make sure the value is the proper type at all times, only when differentiating between Number and Integer. This allows us to drop range checks and allow any Integer and any Number even if they are not representable in Flash Player. This should improve performance in most cases, where we don't need a range check. This introduces overflow checks in op_add and op_subtract, but they should be faster than range checks which happened after each op anyway. --- core/src/avm2/activation.rs | 18 ++++++++++++++---- core/src/avm2/value.rs | 24 ++++++++++-------------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index 27a1bdace686..af5da0686be0 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -1962,8 +1962,13 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value1 = self.pop_stack(); let sum_value = match (value1, value2) { - // note: with not-yet-guaranteed assumption that Integer < 1<<28, this won't overflow. - (Value::Integer(n1), Value::Integer(n2)) => (n1 + n2).into(), + (Value::Integer(n1), Value::Integer(n2)) => { + if let Some(res) = n1.checked_add(n2) { + res.into() + } else { + (n1 as f64 + n2 as f64).into() + } + } (Value::Number(n1), Value::Number(n2)) => (n1 + n2).into(), (Value::String(s), value2) => Value::String(AvmString::concat( self.gc(), @@ -2208,8 +2213,13 @@ impl<'a, 'gc> Activation<'a, 'gc> { let value1 = self.pop_stack(); let sub_value: Value<'gc> = match (value1, value2) { - // note: with not-yet-guaranteed assumption that Integer < 1<<28, this won't underflow. - (Value::Integer(n1), Value::Integer(n2)) => (n1 - n2).into(), + (Value::Integer(n1), Value::Integer(n2)) => { + if let Some(res) = n1.checked_sub(n2) { + res.into() + } else { + (n1 as f64 - n2 as f64).into() + } + } (Value::Number(n1), Value::Number(n2)) => (n1 - n2).into(), _ => { let value2 = value2.coerce_to_number(self)?; diff --git a/core/src/avm2/value.rs b/core/src/avm2/value.rs index 912849ddc41e..56d2a4bc0dab 100644 --- a/core/src/avm2/value.rs +++ b/core/src/avm2/value.rs @@ -41,11 +41,15 @@ pub enum Value<'gc> { Undefined, Null, Bool(bool), + + /// Numbers that are representable as ints are equivalent to + /// [`Value::Integer`]. See [`Value::normalize`] for details. Number(f64), - // note: this value should never reach +/- 1<<28; this is currently not enforced (TODO). - // Ruffle currently won't break if you break that invariant, - // but some FP compatibility edge cases depend on it, so we should do this at some point. + + /// All integers are equivalent to [`Value::Number`]. + /// See [`Value::normalize`] for details. Integer(i32), + String(AvmString<'gc>), Object(Object<'gc>), } @@ -118,18 +122,14 @@ impl From for Value<'_> { impl From for Value<'_> { fn from(value: i32) -> Self { - if fits_in_value_integer_i32(value) { - Value::Integer(value) - } else { - Value::Number(value as f64) - } + Value::Integer(value) } } impl From for Value<'_> { fn from(value: u32) -> Self { - if fits_in_value_integer_u32(value) { - Value::Integer(value as i32) + if let Some(value) = value.to_i32() { + Value::Integer(value) } else { Value::Number(value as f64) } @@ -163,10 +163,6 @@ fn fits_in_value_integer_i32(value: i32) -> bool { value < (1 << 28) && value >= -(1 << 28) } -fn fits_in_value_integer_u32(value: u32) -> bool { - value < (1 << 28) -} - /// Strips leading whitespace. fn skip_spaces(s: &mut &WStr) { *s = s.trim_start_matches(|c| {