Changeset 63244 in webkit for trunk/JavaScriptCore/bytecompiler

Timestamp:

Jul 13, 2010, 1:34:11 PM (15 years ago)

Author:

[email protected]

Message:

Bug 42182 - Change how numeric compare functions are detected

Reviewed by Oliver Hunt.

JavaScriptCore:

There are three problems with the current mechanism:

• It requires that a function executable be bytecode compiled without being JIT generated (in order to copy the bytecode from the numeric compare function). This is a problem since we have an invariant when running with the JIT that functions are never bytecode compiled without also being JIT generated (after checking the codeblock we assume the function has JIT code). To help maintain this invariant
• This implementation will prevent us from experimenting with alternate compilation paths which do not compile via bytecode.
• It doesn't work. Functions passing more than two arguments will match if they are comparing their last two arguments, not the first two. Generally the mapping back from bytecode to semantics may be more complex then initially expected.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::generate):
(JSC::BytecodeGenerator::setIsNumericCompareFunction):
(JSC::BytecodeGenerator::argumentNumberFor):

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

(JSC::BlockNode::singleStatement):
(JSC::FunctionBodyNode::emitBytecode):

• parser/Nodes.h:

(JSC::ExpressionNode::isSubtract):
(JSC::BinaryOpNode::lhs):
(JSC::BinaryOpNode::rhs):
(JSC::SubNode::isSubtract):
(JSC::ReturnNode::value):

• runtime/JSGlobalData.cpp:

(JSC::JSGlobalData::JSGlobalData):

• runtime/JSGlobalData.h:

LayoutTests:

Test case.

• fast/js/array-sort-numericCompare-expected.txt: Added.
• fast/js/array-sort-numericCompare.html: Added.
• fast/js/script-tests/array-sort-numericCompare.js: Added.

(doSort):
(dontSort):

Location:

trunk/JavaScriptCore/bytecompiler

Files:

• BytecodeGenerator.cpp (modified) (2 diffs)
• BytecodeGenerator.h (modified) (1 diff)
• NodesCodegen.cpp (modified) (2 diffs)

trunk/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -153 +153 @@
     if ((m_codeType == FunctionCode && !m_codeBlock->needsFullScopeChain() && !m_codeBlock->usesArguments()) || m_codeType == EvalCode)
         symbolTable().clear();
-        
-    m_codeBlock->setIsNumericCompareFunction(instructions() == m_globalData->numericCompareFunction(m_scopeChain->globalObject()->globalExec()));
 
 #if !ENABLE(OPCODE_SAMPLING)
@@ -2046 +2044 @@
 }
 
+void BytecodeGenerator::setIsNumericCompareFunction(bool isNumericCompareFunction)
+{
+    m_codeBlock->setIsNumericCompareFunction(isNumericCompareFunction);
+}
+
+int BytecodeGenerator::argumentNumberFor(const Identifier& ident)
+{
+    int parameterCount = m_parameters.size(); // includes 'this'
+    int index = registerFor(ident)->index() + RegisterFile::CallFrameHeaderSize + parameterCount;
+    return (index > 0 && index < parameterCount) ? index : 0;
+}
+
 } // namespace JSC

trunk/JavaScriptCore/bytecompiler/BytecodeGenerator.h

@@ -109 +109 @@
         // require explicit reference counting.
         RegisterID* registerFor(const Identifier&);
-        
+
+        // Returns the agument number if this is an argument, or 0 if not.
+        int argumentNumberFor(const Identifier&);
+
+        void setIsNumericCompareFunction(bool isNumericCompareFunction);
+
         bool willResolveToArguments(const Identifier&);
         RegisterID* uncheckedRegisterForArguments();

trunk/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -1360 +1360 @@
 }
 
+inline StatementNode* BlockNode::singleStatement() const
+{
+    return m_statements ? m_statements->singleStatement() : 0;
+}
+
 RegisterID* BlockNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
@@ -2012 +2017 @@
     generator.emitDebugHook(DidEnterCallFrame, firstLine(), lastLine());
     emitStatementsBytecode(generator, generator.ignoredResult());
+
     StatementNode* singleStatement = this->singleStatement();
+    ReturnNode* returnNode = 0;
+
+    // Check for a return statement at the end of a function composed of a single block.
     if (singleStatement && singleStatement->isBlock()) {
         StatementNode* lastStatementInBlock = static_cast<BlockNode*>(singleStatement)->lastStatement();
         if (lastStatementInBlock && lastStatementInBlock->isReturnNode())
-            return 0;
-    }
-
-    RegisterID* r0 = generator.isConstructor() ? generator.thisRegister() : generator.emitLoad(0, jsUndefined());
-    generator.emitDebugHook(WillLeaveCallFrame, firstLine(), lastLine());
-    generator.emitReturn(r0);
+            returnNode = static_cast<ReturnNode*>(lastStatementInBlock);
+    }
+
+    // If there is no return we must automatically insert one.
+    if (!returnNode) {
+        RegisterID* r0 = generator.isConstructor() ? generator.thisRegister() : generator.emitLoad(0, jsUndefined());
+        generator.emitDebugHook(WillLeaveCallFrame, firstLine(), lastLine());
+        generator.emitReturn(r0);
+        return 0;
+    }
+
+    // If there is a return statment, and it is the only statement in the function, check if this is a numeric compare.
+    if (returnNode && static_cast<BlockNode*>(singleStatement)->singleStatement()) {
+        ExpressionNode* returnValueExpression = returnNode->value();
+        if (returnValueExpression && returnValueExpression->isSubtract()) {
+            ExpressionNode* lhsExpression = static_cast<SubNode*>(returnValueExpression)->lhs();
+            ExpressionNode* rhsExpression = static_cast<SubNode*>(returnValueExpression)->rhs();
+            if (lhsExpression->isResolveNode() && rhsExpression->isResolveNode()) {
+                generator.setIsNumericCompareFunction(generator.argumentNumberFor(static_cast<ResolveNode*>(lhsExpression)->identifier()) == 1
+                    && generator.argumentNumberFor(static_cast<ResolveNode*>(rhsExpression)->identifier()) == 2);
+            }
+        }
+    }
+
     return 0;
 }