[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
barraclough at apple.com
barraclough at apple.com
Wed Dec 22 11:09:03 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit b467c4c6a693b9e17709728106e2712e4b7d9633
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Jul 13 20:34:11 2010 +0000
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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@63244 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index f7a6dc8..eefcf40 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,41 @@
+2010-07-13 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Bug 42182 - Change how numeric compare functions are detected
+
+ 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:
+
2010-07-12 Oliver Hunt <oliver at apple.com>
Reviewed by Gavin Barraclough.
diff --git a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
index 8ff1b5d..ff8a9c6 100644
--- a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
+++ b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -152,8 +152,6 @@ void BytecodeGenerator::generate()
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)
if (!m_regeneratingForExceptionInfo && !m_usesExceptions && (m_codeType == FunctionCode || m_codeType == EvalCode))
@@ -2045,4 +2043,16 @@ RegisterID* BytecodeGenerator::emitThrowExpressionTooDeepException()
return exception;
}
+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
diff --git a/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/JavaScriptCore/bytecompiler/BytecodeGenerator.h
index 2b231a7..ad0ae4e 100644
--- a/JavaScriptCore/bytecompiler/BytecodeGenerator.h
+++ b/JavaScriptCore/bytecompiler/BytecodeGenerator.h
@@ -108,7 +108,12 @@ namespace JSC {
// such register exists. Registers returned by registerFor do not
// 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();
diff --git a/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/JavaScriptCore/bytecompiler/NodesCodegen.cpp
index e50ce2d..1337ab7 100644
--- a/JavaScriptCore/bytecompiler/NodesCodegen.cpp
+++ b/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -1359,6 +1359,11 @@ inline StatementNode* BlockNode::lastStatement() const
return m_statements ? m_statements->lastStatement() : 0;
}
+inline StatementNode* BlockNode::singleStatement() const
+{
+ return m_statements ? m_statements->singleStatement() : 0;
+}
+
RegisterID* BlockNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
{
if (m_statements)
@@ -2011,16 +2016,38 @@ RegisterID* FunctionBodyNode::emitBytecode(BytecodeGenerator& generator, Registe
{
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;
+ 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);
+ }
+ }
}
- RegisterID* r0 = generator.isConstructor() ? generator.thisRegister() : generator.emitLoad(0, jsUndefined());
- generator.emitDebugHook(WillLeaveCallFrame, firstLine(), lastLine());
- generator.emitReturn(r0);
return 0;
}
diff --git a/JavaScriptCore/parser/Nodes.cpp b/JavaScriptCore/parser/Nodes.cpp
index ffea524..c41d735 100644
--- a/JavaScriptCore/parser/Nodes.cpp
+++ b/JavaScriptCore/parser/Nodes.cpp
@@ -67,7 +67,7 @@ void SourceElements::append(StatementNode* statement)
m_statements.append(statement);
}
-inline StatementNode* SourceElements::singleStatement() const
+StatementNode* SourceElements::singleStatement() const
{
size_t size = m_statements.size();
return size == 1 ? m_statements[0] : 0;
diff --git a/JavaScriptCore/parser/Nodes.h b/JavaScriptCore/parser/Nodes.h
index 6206384..d25079b 100644
--- a/JavaScriptCore/parser/Nodes.h
+++ b/JavaScriptCore/parser/Nodes.h
@@ -152,6 +152,7 @@ namespace JSC {
virtual bool isCommaNode() const { return false; }
virtual bool isSimpleArray() const { return false; }
virtual bool isAdd() const { return false; }
+ virtual bool isSubtract() const { return false; }
virtual bool hasConditionContextCodegen() const { return false; }
virtual void emitBytecodeInConditionContext(BytecodeGenerator&, Label*, Label*, bool) { ASSERT_NOT_REACHED(); }
@@ -806,6 +807,9 @@ namespace JSC {
RegisterID* emitStrcat(BytecodeGenerator& generator, RegisterID* destination, RegisterID* lhs = 0, ReadModifyResolveNode* emitExpressionInfoForMe = 0);
+ ExpressionNode* lhs() { return m_expr1; };
+ ExpressionNode* rhs() { return m_expr2; };
+
private:
virtual RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0);
@@ -854,6 +858,8 @@ namespace JSC {
class SubNode : public BinaryOpNode {
public:
SubNode(JSGlobalData*, ExpressionNode* expr1, ExpressionNode* expr2, bool rightHasAssignments);
+
+ virtual bool isSubtract() const { return true; }
};
class LeftShiftNode : public BinaryOpNode {
@@ -1143,6 +1149,7 @@ namespace JSC {
public:
BlockNode(JSGlobalData*, SourceElements* = 0);
+ StatementNode* singleStatement() const;
StatementNode* lastStatement() const;
private:
@@ -1294,6 +1301,8 @@ namespace JSC {
public:
ReturnNode(JSGlobalData*, ExpressionNode* value);
+ ExpressionNode* value() { return m_value; }
+
private:
virtual RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0);
diff --git a/JavaScriptCore/runtime/JSGlobalData.cpp b/JavaScriptCore/runtime/JSGlobalData.cpp
index 1508750..b23606e 100644
--- a/JavaScriptCore/runtime/JSGlobalData.cpp
+++ b/JavaScriptCore/runtime/JSGlobalData.cpp
@@ -140,7 +140,6 @@ JSGlobalData::JSGlobalData(GlobalDataType globalDataType, ThreadStackType thread
, jitStubs(this)
#endif
, heap(this)
- , initializingLazyNumericCompareFunction(false)
, head(0)
, dynamicGlobalObject(0)
, functionCodeBlockBeingReparsed(0)
@@ -257,19 +256,6 @@ JSGlobalData*& JSGlobalData::sharedInstanceInternal()
return sharedInstance;
}
-// FIXME: We can also detect forms like v1 < v2 ? -1 : 0, reverse comparison, etc.
-const Vector<Instruction>& JSGlobalData::numericCompareFunction(ExecState* exec)
-{
- if (!lazyNumericCompareFunction.size() && !initializingLazyNumericCompareFunction) {
- initializingLazyNumericCompareFunction = true;
- RefPtr<FunctionExecutable> function = FunctionExecutable::fromGlobalCode(Identifier(exec, "numericCompare"), exec, 0, makeSource(UString("(function (v1, v2) { return v1 - v2; })")), 0, 0);
- lazyNumericCompareFunction = function->bytecodeForCall(exec, exec->scopeChain())->instructions();
- initializingLazyNumericCompareFunction = false;
- }
-
- return lazyNumericCompareFunction;
-}
-
#if ENABLE(JIT)
PassRefPtr<NativeExecutable> JSGlobalData::getHostFunction(NativeFunction function)
{
diff --git a/JavaScriptCore/runtime/JSGlobalData.h b/JavaScriptCore/runtime/JSGlobalData.h
index f3f6cba..bbb1e31 100644
--- a/JavaScriptCore/runtime/JSGlobalData.h
+++ b/JavaScriptCore/runtime/JSGlobalData.h
@@ -195,10 +195,6 @@ namespace JSC {
ReturnAddressPtr exceptionLocation;
#endif
- const Vector<Instruction>& numericCompareFunction(ExecState*);
- Vector<Instruction> lazyNumericCompareFunction;
- bool initializingLazyNumericCompareFunction;
-
HashMap<OpaqueJSClass*, OpaqueJSClassContextData*> opaqueJSClassData;
JSGlobalObject* head;
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e3ab66a..972851a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2010-07-13 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Bug 42182 - Change how numeric compare functions are detected
+
+ 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):
+
2010-07-13 Eric Carlson <eric.carlson at apple.com>
Reviewed by Dan Bernstein.
diff --git a/LayoutTests/fast/js/array-sort-numericCompare-expected.txt b/LayoutTests/fast/js/array-sort-numericCompare-expected.txt
new file mode 100644
index 0000000..a17a715
--- /dev/null
+++ b/LayoutTests/fast/js/array-sort-numericCompare-expected.txt
@@ -0,0 +1,11 @@
+This tests that a call to array.sort(compareFunction) works correctly for numeric comparisons (arg1 - arg2), and also for things that might look like numeric comparisons.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS [3,1,5,2,4].sort(doSort) is [1,2,3,4,5]
+PASS [3,1,5,2,4].sort(dontSort) is [3,1,5,2,4]
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/array-sort-numericCompare.html b/LayoutTests/fast/js/array-sort-numericCompare.html
new file mode 100644
index 0000000..44124f9
--- /dev/null
+++ b/LayoutTests/fast/js/array-sort-numericCompare.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/array-sort-numericCompare.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/script-tests/array-sort-numericCompare.js b/LayoutTests/fast/js/script-tests/array-sort-numericCompare.js
new file mode 100644
index 0000000..1bcc67e
--- /dev/null
+++ b/LayoutTests/fast/js/script-tests/array-sort-numericCompare.js
@@ -0,0 +1,18 @@
+description(
+"This tests that a call to array.sort(compareFunction) works correctly for numeric comparisons (arg1 - arg2), and also for things that might look like numeric comparisons."
+);
+
+function doSort(x, y)
+{
+ return x - y;
+}
+
+function dontSort(w, x, y)
+{
+ return x - y;
+}
+
+shouldBe("[3,1,5,2,4].sort(doSort)", "[1,2,3,4,5]");
+shouldBe("[3,1,5,2,4].sort(dontSort)", "[3,1,5,2,4]");
+
+var successfullyParsed = true;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list