Changeset 249117 in webkit

Timestamp:

Aug 26, 2019, 2:09:35 PM (7 years ago)

Author:

Ross Kirsling

Message:

[JSC] Ensure x?.y ?? z is fast
​https://bugs.webkit.org/show_bug.cgi?id=200875

Reviewed by Yusuke Suzuki.

JSTests:

• stress/nullish-coalescing.js:

Source/JavaScriptCore:

We anticipate x?.y ?? z to quickly become a common idiom in JS. With a little bytecode rearrangement,
we can avoid the "load undefined and check it" dance in the middle and just turn this into two jumps.

Before:

(get x)

----- jundefined_or_null
| (get y)
| --- jmp

| (load undefined)

• jnundefined_or_null

| (get z)

end

After:

(get x)

--- jundefined_or_null
| (get y)
| - jnundefined_or_null

| (get z)
end

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::popOptionalChainTarget): Added specialization.

• bytecompiler/BytecodeGenerator.h:
• bytecompiler/NodesCodegen.cpp:

(JSC::CoalesceNode::emitBytecode):
(JSC::OptionalChainNode::emitBytecode):

• parser/ASTBuilder.h:

(JSC::ASTBuilder::makeDeleteNode):
(JSC::ASTBuilder::makeCoalesceNode): Added.
(JSC::ASTBuilder::makeBinaryNode):

• parser/NodeConstructors.h:

(JSC::CoalesceNode::CoalesceNode):

• parser/Nodes.h:

(JSC::ExpressionNode::isDeleteNode const): Added. (Replaces OptionalChainNode::m_isDelete.)

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/nullish-coalescing.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (1 diff)
• Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h (modified) (1 diff)
• Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp (modified) (2 diffs)
• Source/JavaScriptCore/parser/ASTBuilder.h (modified) (4 diffs)
• Source/JavaScriptCore/parser/NodeConstructors.h (modified) (1 diff)
• Source/JavaScriptCore/parser/Nodes.h (modified) (9 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-08-26  Ross Kirsling  <ross.kirsling@sony.com>
+
+        [JSC] Ensure x?.y ?? z is fast
+        https://bugs.webkit.org/show_bug.cgi?id=200875
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/nullish-coalescing.js:
+
 2019-08-23  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/JSTests/stress/nullish-coalescing.js

@@ -90 +90 @@
 shouldThrowSyntaxError('0 || 1 && 2 | 3 ^ 4 & 5 == 6 != 7 === 8 !== 9 < 0 > 1 <= 2 >= 3 << 4 >> 5 >>> 6 + 7 - 8 * 9 / 0 % 1 ** 2 ?? 3');
 shouldThrowSyntaxError('3 ?? 2 ** 1 % 0 / 9 * 8 - 7 + 6 >>> 5 >> 4 << 3 >= 2 <= 1 > 0 < 9 !== 8 === 7 != 6 == 5 & 4 ^ 3 | 2 && 1 || 0');
+
+shouldBe(null?.x ?? 3, 3);
+shouldBe(({})?.x ?? 3, 3);
+shouldBe(({ x: 0 })?.x ?? 3, 0);
+shouldBe(null?.() ?? 3, 3);
+shouldBe((() => 0)?.() ?? 3, 0);
+shouldBe(({ x: 0 })?.[null?.a ?? 'x'] ?? 3, 0);
+shouldBe((() => 0)?.(null?.a ?? 'x') ?? 3, 0);

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-08-26  Ross Kirsling  <ross.kirsling@sony.com>
+
+        [JSC] Ensure x?.y ?? z is fast
+        https://bugs.webkit.org/show_bug.cgi?id=200875
+
+        Reviewed by Yusuke Suzuki.
+
+        We anticipate `x?.y ?? z` to quickly become a common idiom in JS. With a little bytecode rearrangement,
+        we can avoid the "load undefined and check it" dance in the middle and just turn this into two jumps.
+
+        Before:
+                (get x)
+          ----- jundefined_or_null
+          |     (get y)
+          | --- jmp
+          > |   (load undefined)
+            > - jnundefined_or_null
+              | (get z)
+              > end
+
+        After:
+                (get x)
+            --- jundefined_or_null
+            |   (get y)
+            | - jnundefined_or_null
+            > | (get z)
+              > end
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::popOptionalChainTarget): Added specialization.
+        * bytecompiler/BytecodeGenerator.h:
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::CoalesceNode::emitBytecode):
+        (JSC::OptionalChainNode::emitBytecode):
+        * parser/ASTBuilder.h:
+        (JSC::ASTBuilder::makeDeleteNode):
+        (JSC::ASTBuilder::makeCoalesceNode): Added.
+        (JSC::ASTBuilder::makeBinaryNode):
+        * parser/NodeConstructors.h:
+        (JSC::CoalesceNode::CoalesceNode):
+        * parser/Nodes.h:
+        (JSC::ExpressionNode::isDeleteNode const): Added. (Replaces OptionalChainNode::m_isDelete.)
+
 2019-08-26  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -4970 +4970 @@
 }
 
+void BytecodeGenerator::popOptionalChainTarget()
+{
+    ASSERT(m_optionalChainTargetStack.size());
+    emitLabel(m_optionalChainTargetStack.takeLast().get());
+}
+
 void BytecodeGenerator::popOptionalChainTarget(RegisterID* dst, bool isDelete)
 {
-    ASSERT(m_optionalChainTargetStack.size());
-
     Ref<Label> endLabel = newLabel();
     emitJump(endLabel.get());
 
-    emitLabel(m_optionalChainTargetStack.takeLast().get());
+    popOptionalChainTarget();
     emitLoad(dst, isDelete ? jsBoolean(true) : jsUndefined());
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

@@ -961 +961 @@
 
         void pushOptionalChainTarget();
-        void popOptionalChainTarget(RegisterID* dst, bool isDelete = false);
+        void popOptionalChainTarget();
+        void popOptionalChainTarget(RegisterID* dst, bool isDelete);
         void emitOptionalCheck(RegisterID* src);
 

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -2409 +2409 @@
 {
     RefPtr<RegisterID> temp = generator.tempDestination(dst);
-    Ref<Label> target = generator.newLabel();
-
+    Ref<Label> endLabel = generator.newLabel();
+
+    if (m_hasAbsorbedOptionalChain)
+        generator.pushOptionalChainTarget();
     generator.emitNode(temp.get(), m_expr1);
-    generator.emitJumpIfFalse(generator.emitIsUndefinedOrNull(generator.newTemporary(), temp.get()), target.get());
+    generator.emitJumpIfFalse(generator.emitIsUndefinedOrNull(generator.newTemporary(), temp.get()), endLabel.get());
+
+    if (m_hasAbsorbedOptionalChain)
+        generator.popOptionalChainTarget();
     generator.emitNodeInTailPosition(temp.get(), m_expr2);
-    generator.emitLabel(target.get());
-
+
+    generator.emitLabel(endLabel.get());
     return generator.move(dst, temp.get());
 }
@@ -2429 +2434 @@
     generator.emitNodeInTailPosition(finalDest.get(), m_expr);
     if (m_isOutermost)
-        generator.popOptionalChainTarget(finalDest.get(), m_isDelete);
+        generator.popOptionalChainTarget(finalDest.get(), m_expr->isDeleteNode());
 
     return finalDest.get();

trunk/Source/JavaScriptCore/parser/ASTBuilder.h

@@ -153 +153 @@
     ExpressionNode* makeBitAndNode(const JSTokenLocation&, ExpressionNode* left, ExpressionNode* right, bool rightHasAssignments);
     ExpressionNode* makeBitOrNode(const JSTokenLocation&, ExpressionNode* left, ExpressionNode* right, bool rightHasAssignments);
+    ExpressionNode* makeCoalesceNode(const JSTokenLocation&, ExpressionNode* left, ExpressionNode* right);
     ExpressionNode* makeLeftShiftNode(const JSTokenLocation&, ExpressionNode* left, ExpressionNode* right, bool rightHasAssignments);
     ExpressionNode* makeRightShiftNode(const JSTokenLocation&, ExpressionNode* left, ExpressionNode* right, bool rightHasAssignments);
@@ -1160 +1161 @@
             ASSERT(!optionalChain->expr()->isResolveNode());
             optionalChain->setExpr(makeDeleteNode(location, optionalChain->expr(), start, divot, end));
-            optionalChain->setIsDelete();
             return optionalChain;
         }
@@ -1360 +1360 @@
     }
     return new (m_parserArena) BitXOrNode(location, expr1, expr2, rightHasAssignments);
+}
+
+ExpressionNode* ASTBuilder::makeCoalesceNode(const JSTokenLocation& location, ExpressionNode* expr1, ExpressionNode* expr2)
+{
+    // Optimization for `x?.y ?? z`.
+    if (expr1->isOptionalChain()) {
+        OptionalChainNode* optionalChain = static_cast<OptionalChainNode*>(expr1);
+        if (!optionalChain->expr()->isDeleteNode()) {
+            constexpr bool hasAbsorbedOptionalChain = true;
+            return new (m_parserArena) CoalesceNode(location, optionalChain->expr(), expr2, hasAbsorbedOptionalChain);
+        }
+    }
+    constexpr bool hasAbsorbedOptionalChain = false;
+    return new (m_parserArena) CoalesceNode(location, expr1, expr2, hasAbsorbedOptionalChain);
 }
 
@@ -1425 +1439 @@
     switch (token) {
     case COALESCE:
-        return new (m_parserArena) CoalesceNode(location, lhs.first, rhs.first);
+        return makeCoalesceNode(location, lhs.first, rhs.first);
 
     case OR:

trunk/Source/JavaScriptCore/parser/NodeConstructors.h

@@ -670 +670 @@
     }
 
-    inline CoalesceNode::CoalesceNode(const JSTokenLocation& location, ExpressionNode* expr1, ExpressionNode* expr2)
+    inline CoalesceNode::CoalesceNode(const JSTokenLocation& location, ExpressionNode* expr1, ExpressionNode* expr2, bool hasAbsorbedOptionalChain)
         : ExpressionNode(location, ResultType::forCoalesce(expr1->resultDescriptor(), expr2->resultDescriptor()))
         , m_expr1(expr1)
         , m_expr2(expr2)
+        , m_hasAbsorbedOptionalChain(hasAbsorbedOptionalChain)
     {
     }

trunk/Source/JavaScriptCore/parser/Nodes.h

@@ -206 +206 @@
         virtual bool isBinaryOpNode() const { return false; }
         virtual bool isFunctionCall() const { return false; }
+        virtual bool isDeleteNode() const { return false; }
         virtual bool isOptionalChain() const { return false; }
 
@@ -995 +996 @@
         RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0) override;
 
+        bool isDeleteNode() const final { return true; }
+
         const Identifier& m_ident;
     };
@@ -1004 +1007 @@
     private:
         RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0) override;
+
+        bool isDeleteNode() const final { return true; }
 
         ExpressionNode* m_base;
@@ -1016 +1021 @@
         RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0) override;
 
+        bool isDeleteNode() const final { return true; }
+
         ExpressionNode* m_base;
         const Identifier& m_ident;
@@ -1026 +1033 @@
     private:
         RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0) override;
+
+        bool isDeleteNode() const final { return true; }
 
         ExpressionNode* m_expr;
@@ -1313 +1322 @@
     class CoalesceNode final : public ExpressionNode {
     public:
-        CoalesceNode(const JSTokenLocation&, ExpressionNode* expr1, ExpressionNode* expr2);
+        CoalesceNode(const JSTokenLocation&, ExpressionNode* expr1, ExpressionNode* expr2, bool);
 
     private:
@@ -1320 +1329 @@
         ExpressionNode* m_expr1;
         ExpressionNode* m_expr2;
+        bool m_hasAbsorbedOptionalChain;
     };
 
@@ -1329 +1339 @@
         ExpressionNode* expr() const { return m_expr; }
 
-        void setIsDelete() { m_isDelete = true; }
-
     private:
         RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = nullptr) final;
@@ -1338 +1346 @@
         ExpressionNode* m_expr;
         bool m_isOutermost;
-        bool m_isDelete { false };
     };