From 17c1f742a9d1849a5cbab5e7293f4c501aeacbf5 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 4 Mar 2024 10:56:21 +0100
Subject: [PATCH] LibJS/Bytecode: Increase coverage of left/shift expression
 fast paths

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.
---
 Userland/Libraries/LibJS/Bytecode/Interpreter.cpp |  6 +++---
 Userland/Libraries/LibJS/Runtime/Value.cpp        | 12 ------------
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
index 271ef250ba..7ac831237e 100644
--- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
@@ -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 {};
@@ -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 {};
@@ -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 {};
diff --git a/Userland/Libraries/LibJS/Runtime/Value.cpp b/Userland/Libraries/LibJS/Runtime/Value.cpp
index 077d7b4254..2ec8e3b65d 100644
--- a/Userland/Libraries/LibJS/Runtime/Value.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Value.cpp
@@ -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.
 
@@ -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.