Skip to content

Commit 81fd313

Browse files
committed
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.
1 parent 2923ff9 commit 81fd313

File tree

2 files changed

+24
-18
lines changed

2 files changed

+24
-18
lines changed

core/src/avm2/activation.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,8 +1962,13 @@ impl<'a, 'gc> Activation<'a, 'gc> {
19621962
let value1 = self.pop_stack();
19631963

19641964
let sum_value = match (value1, value2) {
1965-
// note: with not-yet-guaranteed assumption that Integer < 1<<28, this won't overflow.
1966-
(Value::Integer(n1), Value::Integer(n2)) => (n1 + n2).into(),
1965+
(Value::Integer(n1), Value::Integer(n2)) => {
1966+
if let Some(res) = n1.checked_add(n2) {
1967+
res.into()
1968+
} else {
1969+
(n1 as f64 + n2 as f64).into()
1970+
}
1971+
}
19671972
(Value::Number(n1), Value::Number(n2)) => (n1 + n2).into(),
19681973
(Value::String(s), value2) => Value::String(AvmString::concat(
19691974
self.gc(),
@@ -2208,8 +2213,13 @@ impl<'a, 'gc> Activation<'a, 'gc> {
22082213
let value1 = self.pop_stack();
22092214

22102215
let sub_value: Value<'gc> = match (value1, value2) {
2211-
// note: with not-yet-guaranteed assumption that Integer < 1<<28, this won't underflow.
2212-
(Value::Integer(n1), Value::Integer(n2)) => (n1 - n2).into(),
2216+
(Value::Integer(n1), Value::Integer(n2)) => {
2217+
if let Some(res) = n1.checked_sub(n2) {
2218+
res.into()
2219+
} else {
2220+
(n1 as f64 - n2 as f64).into()
2221+
}
2222+
}
22132223
(Value::Number(n1), Value::Number(n2)) => (n1 - n2).into(),
22142224
_ => {
22152225
let value2 = value2.coerce_to_number(self)?;

core/src/avm2/value.rs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,15 @@ pub enum Value<'gc> {
4141
Undefined,
4242
Null,
4343
Bool(bool),
44+
45+
/// Numbers that are representable as ints are equivalent to
46+
/// [`Value::Integer`]. See [`Value::normalize`] for details.
4447
Number(f64),
45-
// note: this value should never reach +/- 1<<28; this is currently not enforced (TODO).
46-
// Ruffle currently won't break if you break that invariant,
47-
// but some FP compatibility edge cases depend on it, so we should do this at some point.
48+
49+
/// All integers are equivalent to [`Value::Number`].
50+
/// See [`Value::normalize`] for details.
4851
Integer(i32),
52+
4953
String(AvmString<'gc>),
5054
Object(Object<'gc>),
5155
}
@@ -118,18 +122,14 @@ impl From<u16> for Value<'_> {
118122

119123
impl From<i32> for Value<'_> {
120124
fn from(value: i32) -> Self {
121-
if fits_in_value_integer_i32(value) {
122-
Value::Integer(value)
123-
} else {
124-
Value::Number(value as f64)
125-
}
125+
Value::Integer(value)
126126
}
127127
}
128128

129129
impl From<u32> for Value<'_> {
130130
fn from(value: u32) -> Self {
131-
if fits_in_value_integer_u32(value) {
132-
Value::Integer(value as i32)
131+
if let Some(value) = value.to_i32() {
132+
Value::Integer(value)
133133
} else {
134134
Value::Number(value as f64)
135135
}
@@ -163,10 +163,6 @@ fn fits_in_value_integer_i32(value: i32) -> bool {
163163
value < (1 << 28) && value >= -(1 << 28)
164164
}
165165

166-
fn fits_in_value_integer_u32(value: u32) -> bool {
167-
value < (1 << 28)
168-
}
169-
170166
/// Strips leading whitespace.
171167
fn skip_spaces(s: &mut &WStr) {
172168
*s = s.trim_start_matches(|c| {

0 commit comments

Comments
 (0)