Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(minifier): late peeophole optimization #6882

Merged
merged 1 commit into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions crates/oxc_ecmascript/src/constant_evaluation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ mod value_type;
use std::{borrow::Cow, cmp::Ordering};

use num_bigint::BigInt;
use num_traits::{One, Zero};
use num_traits::Zero;

use oxc_ast::ast::*;

use crate::{side_effects::MayHaveSideEffects, ToBigInt, ToInt32, ToJsString, ToNumber};
use crate::{side_effects::MayHaveSideEffects, ToBigInt, ToBoolean, ToInt32, ToJsString, ToNumber};

pub use self::{is_litral_value::IsLiteralValue, value::ConstantValue, value_type::ValueType};

Expand Down Expand Up @@ -50,6 +50,14 @@ pub trait ConstantEvaluation<'a> {
None
}

fn get_side_free_boolean_value(&self, expr: &Expression<'a>) -> Option<bool> {
let value = expr.to_boolean();
if value.is_some() && !expr.may_have_side_effects() {
return value;
}
None
}

fn get_side_free_bigint_value(&self, expr: &Expression<'a>) -> Option<BigInt> {
let value = expr.to_big_int();
if value.is_some() && expr.may_have_side_effects() {
Expand Down Expand Up @@ -102,7 +110,6 @@ pub trait ConstantEvaluation<'a> {
Expression::UnaryExpression(unary_expr) => {
match unary_expr.operator {
UnaryOperator::Void => Some(false),

UnaryOperator::BitwiseNot
| UnaryOperator::UnaryPlus
| UnaryOperator::UnaryNegation => {
Expand Down Expand Up @@ -180,6 +187,8 @@ pub trait ConstantEvaluation<'a> {
Expression::Identifier(ident) => self.resolve_binding(ident),
Expression::NumericLiteral(lit) => Some(ConstantValue::Number(lit.value)),
Expression::NullLiteral(_) => Some(ConstantValue::Null),
Expression::BooleanLiteral(lit) => Some(ConstantValue::Boolean(lit.value)),
Expression::BigIntLiteral(lit) => lit.to_big_int().map(ConstantValue::BigInt),
Expression::StringLiteral(lit) => {
Some(ConstantValue::String(Cow::Borrowed(lit.value.as_str())))
}
Expand Down Expand Up @@ -353,12 +362,6 @@ pub trait ConstantEvaluation<'a> {
None
}
UnaryOperator::LogicalNot => {
// Don't fold !0 and !1 back to false.
if let Expression::NumericLiteral(n) = &expr.argument {
if n.value.is_zero() || n.value.is_one() {
return None;
}
}
self.get_boolean_value(&expr.argument).map(|b| !b).map(ConstantValue::Boolean)
}
UnaryOperator::UnaryPlus => {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_ecmascript/src/constant_evaluation/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ impl<'a> ToJsString<'a> for ConstantValue<'a> {
use oxc_syntax::number::ToJsString;
Some(Cow::Owned(n.to_js_string()))
}
// FIXME: to js number string
Self::BigInt(n) => Some(Cow::Owned(n.to_string() + "n")),
// https://tc39.es/ecma262/#sec-numeric-types-bigint-tostring
Self::BigInt(n) => Some(Cow::Owned(n.to_string())),
Self::String(s) => Some(s.clone()),
Self::Boolean(b) => Some(Cow::Borrowed(if *b { "true" } else { "false" })),
Self::Undefined => Some(Cow::Borrowed("undefined")),
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_ecmascript/src/constant_evaluation/value_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ impl ValueType {
pub fn is_bigint(self) -> bool {
matches!(self, Self::BigInt)
}

pub fn is_boolean(self) -> bool {
matches!(self, Self::Boolean)
}
}

/// `get_known_value_type`
Expand Down
16 changes: 10 additions & 6 deletions crates/oxc_ecmascript/src/to_big_int.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use num_bigint::BigInt;
use num_traits::{One, Zero};

use oxc_ast::ast::Expression;
use oxc_ast::ast::{BigIntLiteral, Expression};
use oxc_syntax::operator::UnaryOperator;

use crate::{StringToBigInt, ToBoolean, ToJsString};
Expand All @@ -25,11 +25,7 @@ impl<'a> ToBigInt<'a> for Expression<'a> {
None
}
}
Expression::BigIntLiteral(bigint_literal) => {
let value = bigint_literal.raw.as_str().trim_end_matches('n').string_to_big_int();
debug_assert!(value.is_some(), "Failed to parse {}", bigint_literal.raw);
value
}
Expression::BigIntLiteral(lit) => lit.to_big_int(),
Expression::BooleanLiteral(bool_literal) => {
if bool_literal.value {
Some(BigInt::one())
Expand Down Expand Up @@ -68,3 +64,11 @@ impl<'a> ToBigInt<'a> for Expression<'a> {
}
}
}

impl<'a> ToBigInt<'a> for BigIntLiteral<'a> {
fn to_big_int(&self) -> Option<BigInt> {
let value = self.raw.as_str().trim_end_matches('n').string_to_big_int();
debug_assert!(value.is_some(), "Failed to parse {}", self.raw);
value
}
}
25 changes: 23 additions & 2 deletions crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,17 @@ impl<'a> Traverse<'a> for PeepholeFoldConstants {
Expression::ArrayExpression(e) => Self::try_flatten_array_expression(e, ctx),
Expression::ObjectExpression(e) => Self::try_flatten_object_expression(e, ctx),
Expression::BinaryExpression(e) => Self::try_fold_binary_expression(e, ctx),
#[allow(clippy::float_cmp)]
Expression::UnaryExpression(e) => {
ctx.eval_unary_expression(e).map(|v| ctx.value_to_expr(e.span, v))
match e.operator {
// Do not fold `void 0` back to `undefined`.
UnaryOperator::Void if e.argument.is_number_0() => None,
// Do not fold `true` and `false` back to `!0` and `!1`
UnaryOperator::LogicalNot if matches!(&e.argument, Expression::NumericLiteral(lit) if lit.value == 0.0 || lit.value == 1.0) => {
None
}
_ => ctx.eval_unary_expression(e).map(|v| ctx.value_to_expr(e.span, v)),
}
}
// TODO: return tryFoldGetProp(subtree);
Expression::LogicalExpression(e) => Self::try_fold_logical_expression(e, ctx),
Expand Down Expand Up @@ -414,7 +423,19 @@ impl<'a, 'b> PeepholeFoldConstants {
Tri::Unknown
}
ValueType::Undefined | ValueType::Null => Tri::True,
_ => Tri::Unknown,
ValueType::Boolean if right.is_boolean() => {
let left = ctx.get_boolean_value(left_expr);
let right = ctx.get_boolean_value(right_expr);
if let (Some(left_bool), Some(right_bool)) = (left, right) {
return Tri::from(left_bool == right_bool);
}
Tri::Unknown
}
// TODO
ValueType::BigInt
| ValueType::Object
| ValueType::Boolean
| ValueType::Undetermined => Tri::Unknown,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@ use crate::{node_util::Ctx, CompressOptions, CompressorPass};
/// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeSubstituteAlternateSyntax.java>
pub struct PeepholeSubstituteAlternateSyntax {
options: CompressOptions,

/// Do not compress syntaxes that are hard to analyze inside the fixed loop.
/// e.g. Do not compress `undefined -> void 0`, `true` -> `!0`.
/// Opposite of `late` in Closure Compier.
in_fixed_loop: bool,

// states
in_define_export: bool,

changed: bool,
}

Expand Down Expand Up @@ -83,13 +91,12 @@ impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax {
self.changed = true;
}
}
if !Self::compress_undefined(expr, ctx) {
self.compress_boolean(expr, ctx);
}
}

fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
let ctx = Ctx(ctx);
self.try_compress_boolean(expr, ctx);
self.try_compress_undefined(expr, ctx);
match expr {
Expression::NewExpression(new_expr) => {
if let Some(new_expr) = Self::try_fold_new_expression(new_expr, ctx) {
Expand Down Expand Up @@ -155,19 +162,21 @@ impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax {
}

impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
pub fn new(options: CompressOptions) -> Self {
Self { options, in_define_export: false, changed: false }
pub fn new(in_fixed_loop: bool, options: CompressOptions) -> Self {
Self { options, in_fixed_loop, in_define_export: false, changed: false }
}

/* Utilities */

/// Transforms `undefined` => `void 0`
fn compress_undefined(expr: &mut Expression<'a>, ctx: Ctx<'a, 'b>) -> bool {
fn try_compress_undefined(&mut self, expr: &mut Expression<'a>, ctx: Ctx<'a, 'b>) {
if self.in_fixed_loop {
return;
}
if ctx.is_expression_undefined(expr) {
*expr = ctx.ast.void_0(expr.span());
return true;
};
false
self.changed = true;
}
}

/// Test `Object.defineProperty(exports, ...)`
Expand Down Expand Up @@ -207,8 +216,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
/// Transforms boolean expression `true` => `!0` `false` => `!1`.
/// Enabled by `compress.booleans`.
/// Do not compress `true` in `Object.defineProperty(exports, 'Foo', {enumerable: true, ...})`.
fn compress_boolean(&mut self, expr: &mut Expression<'a>, ctx: Ctx<'a, 'b>) -> bool {
let Expression::BooleanLiteral(lit) = expr else { return false };
fn try_compress_boolean(&mut self, expr: &mut Expression<'a>, ctx: Ctx<'a, 'b>) {
if self.in_fixed_loop {
return;
}
let Expression::BooleanLiteral(lit) = expr else { return };
if self.options.booleans && !self.in_define_export {
let parent = ctx.ancestry.parent();
let no_unary = {
Expand Down Expand Up @@ -237,9 +249,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
} else {
ctx.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, num)
};
true
} else {
false
self.changed = true;
}
}

Expand Down Expand Up @@ -582,7 +592,8 @@ mod test {

fn test(source_text: &str, expected: &str) {
let allocator = Allocator::default();
let mut pass = super::PeepholeSubstituteAlternateSyntax::new(CompressOptions::default());
let mut pass =
super::PeepholeSubstituteAlternateSyntax::new(false, CompressOptions::default());
tester::test(&allocator, source_text, expected, &mut pass);
}

Expand Down
9 changes: 8 additions & 1 deletion crates/oxc_minifier/src/compressor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ impl<'a> Compressor<'a> {
&mut PeepholeRemoveDeadCode::new(),
// TODO: MinimizeExitPoints
&mut PeepholeMinimizeConditions::new(),
&mut PeepholeSubstituteAlternateSyntax::new(self.options),
&mut PeepholeSubstituteAlternateSyntax::new(
/* in_fixed_loop */ true,
self.options,
),
&mut PeepholeReplaceKnownMethods::new(),
&mut PeepholeFoldConstants::new(),
];
Expand All @@ -75,6 +78,10 @@ impl<'a> Compressor<'a> {
// Passes listed in `getFinalization` in `DefaultPassConfig`
ExploitAssigns::new().build(program, &mut ctx);
CollapseVariableDeclarations::new(self.options).build(program, &mut ctx);

// Late latePeepholeOptimizations
PeepholeSubstituteAlternateSyntax::new(/* in_fixed_loop */ false, self.options)
.build(program, &mut ctx);
}

fn dead_code_elimination(program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
Expand Down
Loading