Skip to content

Commit 5838932

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 5838932

File tree

2 files changed

+17
-15
lines changed

2 files changed

+17
-15
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: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,18 +118,14 @@ impl From<u16> for Value<'_> {
118118

119119
impl From<i32> for Value<'_> {
120120
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-
}
121+
Value::Integer(value)
126122
}
127123
}
128124

129125
impl From<u32> for Value<'_> {
130126
fn from(value: u32) -> Self {
131-
if fits_in_value_integer_u32(value) {
132-
Value::Integer(value as i32)
127+
if let Some(value) = value.to_i32() {
128+
Value::Integer(value)
133129
} else {
134130
Value::Number(value as f64)
135131
}
@@ -163,10 +159,6 @@ fn fits_in_value_integer_i32(value: i32) -> bool {
163159
value < (1 << 28) && value >= -(1 << 28)
164160
}
165161

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

0 commit comments

Comments
 (0)