Skip to content

Commit

Permalink
LibJS/Bytecode: Increase coverage of left/shift expression fast paths
Browse files Browse the repository at this point in the history
As long as the inputs are Int32, we can convert them to UInt32 in a
spec-compliant way with a simple static_cast<u32>.

This allows calculations like `-3 >>> 2` to take the fast path as well,
which is extremely valuable for stuff like crypto code.

While we're doing this, also remove the fast paths from the generic
shift functions in Value.cpp, since we only end up there if we *didn't*
take the same fast path in the interpreter.
  • Loading branch information
awesomekling committed Mar 4, 2024
1 parent a5e1e66 commit 17c1f74
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 15 deletions.
6 changes: 3 additions & 3 deletions Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ ThrowCompletionOr<void> UnsignedRightShift::execute_impl(Bytecode::Interpreter&
auto& vm = interpreter.vm();
auto const lhs = interpreter.get(m_lhs);
auto const rhs = interpreter.get(m_rhs);
if (lhs.is_int32() && rhs.is_int32() && lhs.as_i32() >= 0 && rhs.as_i32() >= 0) {
if (lhs.is_int32() && rhs.is_int32()) {
auto const shift_count = static_cast<u32>(rhs.as_i32()) % 32;
interpreter.set(m_dst, Value(static_cast<u32>(lhs.as_i32()) >> shift_count));
return {};
Expand All @@ -855,7 +855,7 @@ ThrowCompletionOr<void> RightShift::execute_impl(Bytecode::Interpreter& interpre
auto& vm = interpreter.vm();
auto const lhs = interpreter.get(m_lhs);
auto const rhs = interpreter.get(m_rhs);
if (lhs.is_int32() && rhs.is_int32() && rhs.as_i32() >= 0) {
if (lhs.is_int32() && rhs.is_int32()) {
auto const shift_count = static_cast<u32>(rhs.as_i32()) % 32;
interpreter.set(m_dst, Value(lhs.as_i32() >> shift_count));
return {};
Expand All @@ -869,7 +869,7 @@ ThrowCompletionOr<void> LeftShift::execute_impl(Bytecode::Interpreter& interpret
auto& vm = interpreter.vm();
auto const lhs = interpreter.get(m_lhs);
auto const rhs = interpreter.get(m_rhs);
if (lhs.is_int32() && rhs.is_int32() && rhs.as_i32() >= 0) {
if (lhs.is_int32() && rhs.is_int32()) {
auto const shift_count = static_cast<u32>(rhs.as_i32()) % 32;
interpreter.set(m_dst, Value(lhs.as_i32() << shift_count));
return {};
Expand Down
12 changes: 0 additions & 12 deletions Userland/Libraries/LibJS/Runtime/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1598,12 +1598,6 @@ ThrowCompletionOr<Value> left_shift(VM& vm, Value lhs, Value rhs)
// ShiftExpression : ShiftExpression >> AdditiveExpression
ThrowCompletionOr<Value> right_shift(VM& vm, Value lhs, Value rhs)
{
// OPTIMIZATION: Fast path when both values are suitable Int32 values.
if (lhs.is_int32() && rhs.is_int32() && rhs.as_i32() >= 0) {
auto shift_count = static_cast<u32>(rhs.as_i32()) % 32;
return Value(lhs.as_i32() >> shift_count);
}

// 13.15.3 ApplyStringOrNumericBinaryOperator ( lval, opText, rval ), https://tc39.es/ecma262/#sec-applystringornumericbinaryoperator
// 1-2, 6. N/A.

Expand Down Expand Up @@ -1655,12 +1649,6 @@ ThrowCompletionOr<Value> right_shift(VM& vm, Value lhs, Value rhs)
// ShiftExpression : ShiftExpression >>> AdditiveExpression
ThrowCompletionOr<Value> unsigned_right_shift(VM& vm, Value lhs, Value rhs)
{
// OPTIMIZATION: Fast path when both values are suitable Int32 values.
if (lhs.is_int32() && rhs.is_int32() && lhs.as_i32() >= 0 && rhs.as_i32() >= 0) {
auto shift_count = static_cast<u32>(rhs.as_i32()) % 32;
return Value(static_cast<u32>(lhs.as_i32()) >> shift_count);
}

// 13.15.3 ApplyStringOrNumericBinaryOperator ( lval, opText, rval ), https://tc39.es/ecma262/#sec-applystringornumericbinaryoperator
// 1-2, 5-6. N/A.

Expand Down

0 comments on commit 17c1f74

Please sign in to comment.