Changeset 271121 in webkit for trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

Timestamp:

Jan 2, 2021, 12:46:24 PM (4 years ago)

Author:

[email protected]

Message:

[JSC] Remove unnecessary mov bytecodes when performing simple object pattern destructuring to variables
​https://bugs.webkit.org/show_bug.cgi?id=220219

Reviewed by Alexey Shvayka.

JSTests:

• stress/object-pattern-simple-fast-path.js: Added.

(shouldBe):
(shouldThrow):
(test1):

Source/JavaScriptCore:

Currently, we are first puts object pattern's expression into temporary variable, and then, we store it into local variable register.

The following code

({ data } = object);

emits this kind of bytecode.

get_by_id dst:loc10, base:loc9, property:0
mov dst:loc6, src:loc10

However, this should be

get_by_id dst:loc6, base:loc9, property:0

We are emitting many unnecessary movs since this destructuring pattern is common. Increasing amount of mov (1) discourages inlining unnecessarily and (2) simply makes
bytecode memory large. Since this is very common pattern, we should carefully optimize it to remove such unnecessary movs.

This patch looks into pattern when performing object pattern destructuring. And avoid emitting mov when it is possible. There are some cases we cannot remove movs, so
this patch's writableDirectBindingIfPossible looks into whether this is possible (& profitable).

• bytecompiler/NodesCodegen.cpp:

(JSC::ObjectPatternNode::bindValue const):
(JSC::BindingNode::writableDirectBindingIfPossible const):
(JSC::BindingNode::finishDirectBindingAssignment const):
(JSC::AssignmentElementNode::writableDirectBindingIfPossible const):
(JSC::AssignmentElementNode::finishDirectBindingAssignment const):

• parser/Nodes.h:

(JSC::DestructuringPatternNode::writableDirectBindingIfPossible const):
(JSC::DestructuringPatternNode::finishDirectBindingAssignment const):

File:

• trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -5265 +5265 @@
         const auto& target = m_targetPatterns[i];
         if (target.bindingType == BindingType::Element) {
-            RefPtr<RegisterID> temp = generator.newTemporary();
+            // If the destructuring becomes get_by_id and mov, then we should store results directly to the local's binding.
+            // From
+            //     get_by_id          dst:loc10, base:loc9, property:0
+            //     mov                dst:loc6, src:loc10
+            // To
+            //     get_by_id          dst:loc6, base:loc9, property:0
+            auto writableDirectBindingIfPossible = [&]() -> RegisterID* {
+                // The following pattern is possible. In that case, after setting |data| local variable, we need to store property name into the set.
+                // So, old property name |data| result must be kept before setting it into |data|.
+                //     ({ [data]: data, ...obj } = object);
+                if (m_containsRestElement && m_containsComputedProperty && target.propertyExpression)
+                    return nullptr;
+                // default value can include a reference to local variable. So filling value to a local variable can differ result.
+                // We give up fast path if default value includes non constant.
+                // For example,
+                //     ({ data = data } = object);
+                if (target.defaultValue && !target.defaultValue->isConstant())
+                    return nullptr;
+                return target.pattern->writableDirectBindingIfPossible(generator);
+            };
+
+            auto finishDirectBindingAssignment = [&]() {
+                ASSERT(writableDirectBindingIfPossible());
+                target.pattern->finishDirectBindingAssignment(generator);
+            };
+
+            RefPtr<RegisterID> temp;
+            RegisterID* directBinding = writableDirectBindingIfPossible();
+            if (directBinding)
+                temp = directBinding;
+            else
+                temp = generator.newTemporary();
+
             RefPtr<RegisterID> propertyName;
             if (!target.propertyExpression) {
@@ -5297 +5329 @@
             if (target.defaultValue)
                 assignDefaultValueIfUndefined(generator, temp.get(), target.defaultValue);
-            target.pattern->bindValue(generator, temp.get());
+            if (directBinding)
+                finishDirectBindingAssignment();
+            else
+                target.pattern->bindValue(generator, temp.get());
         } else {
             ASSERT(target.bindingType == BindingType::RestElement);
@@ -5332 +5367 @@
 }
 
+RegisterID* BindingNode::writableDirectBindingIfPossible(BytecodeGenerator& generator) const
+{
+    Variable var = generator.variable(m_boundProperty);
+    bool isReadOnly = var.isReadOnly() && m_bindingContext != AssignmentContext::ConstDeclarationStatement;
+    if (RegisterID* local = var.local()) {
+        if (m_bindingContext == AssignmentContext::AssignmentExpression) {
+            if (generator.needsTDZCheck(var))
+                return nullptr;
+        }
+        if (isReadOnly)
+            return nullptr;
+        return local;
+    }
+    return nullptr;
+}
+
+void BindingNode::finishDirectBindingAssignment(BytecodeGenerator& generator) const
+{
+    ASSERT(writableDirectBindingIfPossible(generator));
+    Variable var = generator.variable(m_boundProperty);
+    RegisterID* local = var.local();
+    generator.emitProfileType(local, var, divotStart(), divotEnd());
+    if (m_bindingContext == AssignmentContext::DeclarationStatement || m_bindingContext == AssignmentContext::ConstDeclarationStatement)
+        generator.liftTDZCheckIfPossible(var);
+}
+
 void BindingNode::bindValue(BytecodeGenerator& generator, RegisterID* value) const
 {
@@ -5374 +5435 @@
 {
     identifiers.append(m_boundProperty);
+}
+
+RegisterID* AssignmentElementNode::writableDirectBindingIfPossible(BytecodeGenerator& generator) const
+{
+    if (!m_assignmentTarget->isResolveNode())
+        return nullptr;
+    ResolveNode* lhs = static_cast<ResolveNode*>(m_assignmentTarget);
+    Variable var = generator.variable(lhs->identifier());
+    bool isReadOnly = var.isReadOnly();
+    if (RegisterID* local = var.local()) {
+        if (generator.needsTDZCheck(var))
+            return nullptr;
+        if (isReadOnly)
+            return nullptr;
+        return local;
+    }
+    return nullptr;
+}
+
+void AssignmentElementNode::finishDirectBindingAssignment(BytecodeGenerator& generator) const
+{
+    ASSERT_UNUSED(generator, writableDirectBindingIfPossible(generator));
+    ResolveNode* lhs = static_cast<ResolveNode*>(m_assignmentTarget);
+    Variable var = generator.variable(lhs->identifier());
+    RegisterID* local = var.local();
+    generator.emitProfileType(local, divotStart(), divotEnd());
 }