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

Timestamp:

May 30, 2020, 8:20:40 PM (5 years ago)

Author:

[email protected]

Message:

[JSC] for-in should allocate new temporary register for base
​https://bugs.webkit.org/show_bug.cgi?id=212519
<rdar://problem/63722044>

Reviewed by Saam Barati.

JSTests:

• microbenchmarks/has-own-property-for-in-loop-with-heap-variable.js: Added.

(assert):
(test1.count):
(test1):

• microbenchmarks/has-own-property-for-in-loop-with-this.js: Added.

(assert):
(test1.count):
(test1):

• stress/for-in-body-replace-enumerable.js: Added.

(foo):

• stress/for-in-enumerable-shadow.js: Added.

(assert):
(test1.count):
(test1):

• stress/for-in-enumerable-this-arrow.js: Added.

(assert):
(test1):

Source/JavaScriptCore:

While r262233 keeps for-in's enumerated object in variable register if possible to use this register for heuristics driving an optimization,
for-in body can replace the content of this register during enumeration and confuse enumerator.

Instead, we record Variable information in StructureForInContext. This allows us to detect patterns using heap-variables too.
Further, this patch extends pattern-matching code to support ThisNode too.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::pushStructureForInScope):

• bytecompiler/BytecodeGenerator.h:

(JSC::Variable::Variable):
(JSC::Variable::isResolved const):
(JSC::Variable::symbolTableConstantIndex const):
(JSC::Variable::ident const):
(JSC::Variable::offset const):
(JSC::Variable::isLocal const):
(JSC::Variable::local const):
(JSC::Variable::isReadOnly const):
(JSC::Variable::isSpecial const):
(JSC::Variable::isConst const):
(JSC::Variable::setIsReadOnly):
(JSC::Variable::operator== const):
(JSC::StructureForInContext::StructureForInContext):
(JSC::StructureForInContext::baseVariable const):
(JSC::StructureForInContext::base const): Deleted.

• bytecompiler/NodesCodegen.cpp:

(JSC::HasOwnPropertyFunctionCallDotNode::emitBytecode):
(JSC::ForInNode::emitBytecode):

• parser/ASTBuilder.h:

(JSC::ASTBuilder::makeFunctionCallNode):

• parser/Nodes.h:

(JSC::ExpressionNode::isThisNode const):

File:

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

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -1853 +1853 @@
     }
 
-    if (structureContext && structureContext->base() == base.get()) {
+    auto canUseFastHasOwnProperty = [&] {
+        if (!structureContext)
+            return false;
+        if (!structureContext->baseVariable())
+            return false;
+        if (m_base->isResolveNode())
+            return generator.variable(static_cast<ResolveNode*>(m_base)->identifier()) == structureContext->baseVariable().value();
+        if (m_base->isThisNode()) {
+            // After generator.ensureThis (which must be invoked in |base|'s materialization), we can ensure that |this| is in local this-register.
+            ASSERT(base);
+            return generator.variable(generator.propertyNames().thisIdentifier, ThisResolutionType::Local) == structureContext->baseVariable().value();
+        }
+        return false;
+    };
+
+    if (canUseFastHasOwnProperty()) {
+        // It is possible that base register is variable and each for-in body replaces JS object in the base register with a different one.
+        // Even though, this is OK since HasOwnStructureProperty will reject the replaced JS object.
         Ref<Label> realCall = generator.newLabel();
         Ref<Label> end = generator.newLabel();
@@ -3748 +3765 @@
         generator.emitNode(generator.ignoredResult(), m_lexpr);
 
+    RefPtr<RegisterID> base = generator.newTemporary();
     RefPtr<RegisterID> length;
     RefPtr<RegisterID> enumerator;
 
-    RefPtr<RegisterID> base = generator.emitNode(m_expr);
+    generator.emitNode(base.get(), m_expr);
     RefPtr<RegisterID> local = this->tryGetBoundLocal(generator);
     RefPtr<RegisterID> enumeratorIndex;
+
+    Optional<Variable> baseVariable;
+    if (m_expr->isResolveNode())
+        baseVariable = generator.variable(static_cast<ResolveNode*>(m_expr)->identifier());
+    else if (m_expr->isThisNode()) {
+        // After generator.ensureThis (which must be invoked in |base|'s materialization), we can ensure that |this| is in local this-register.
+        ASSERT(base);
+        baseVariable = generator.variable(generator.propertyNames().thisIdentifier, ThisResolutionType::Local);
+    }
 
     // Pause at the assignment expression for each for..in iteration.
@@ -3829 +3856 @@
         generator.emitProfileControlFlow(profilerStartOffset);
 
-        generator.pushStructureForInScope(local.get(), enumeratorIndex.get(), propertyName.get(), enumerator.get(), base.get());
+        generator.pushStructureForInScope(local.get(), enumeratorIndex.get(), propertyName.get(), enumerator.get(), baseVariable);
         generator.emitNode(dst, m_statement);
         generator.popStructureForInScope(local.get());