rendered paste bodycommit 9b5c619a686f6f952b3ef5e4b3ec0cd42e029aebAuthor: mjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>Date: Wed Oct 8 10:53:13 2008 +0000 2008-10-08 Maciej Stachowiak <mjs@apple.com> Reviewed by Oliver Hunt. Re-landing the following fix with the crashing bug in it fixed (r37405): - optimize away multiplication by constant 1.0 2.3% speedup on v8 RayTrace benchmark Apparently it's not uncommon for JavaScript code to multiply by constant 1.0 in the mistaken belief that this converts integer to floating point and that there is any operational difference. * VM/CTI.cpp: (JSC::CTI::privateCompileMainPass): Optimize to_jsnumber for case where parameter is already number. (JSC::CTI::privateCompileSlowCases): ditto * VM/Machine.cpp: (JSC::Machine::privateExecute): ditto * kjs/grammar.y: (makeMultNode): Transform as follows: +FOO * BAR ==> FOO * BAR FOO * +BAR ==> FOO * BAR FOO * 1 ==> +FOO 1 * FOO ==> +FOO (makeDivNode): Transform as follows: +FOO / BAR ==> FOO / BAR FOO / +BAR ==> FOO / BAR (makeSubNode): Transform as follows: +FOO - BAR ==> FOO - BAR FOO - +BAR ==> FOO - BAR * kjs/nodes.h: (JSC::ExpressionNode::stripUnaryPlus): Helper for above grammar.y changes (JSC::UnaryPlusNode::stripUnaryPlus): ditto git-svn-id: http://svn.webkit.org/repository/webkit/trunk@37417 268f45cc-cd09-0410-ab3c-d52691b4dbfcdiff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLogindex a7c02a2..0dd0d1a 100644--- a/JavaScriptCore/ChangeLog+++ b/JavaScriptCore/ChangeLog@@ -1,6 +1,43 @@ 2008-10-08 Maciej Stachowiak <mjs@apple.com> Reviewed by Oliver Hunt.++ Re-landing the following fix with the crashing bug in it fixed (r37405):+ + - optimize away multiplication by constant 1.0+ + 2.3% speedup on v8 RayTrace benchmark++ Apparently it's not uncommon for JavaScript code to multiply by+ constant 1.0 in the mistaken belief that this converts integer to+ floating point and that there is any operational difference.++ * VM/CTI.cpp:+ (JSC::CTI::privateCompileMainPass): Optimize to_jsnumber for+ case where parameter is already number.+ (JSC::CTI::privateCompileSlowCases): ditto+ * VM/Machine.cpp:+ (JSC::Machine::privateExecute): ditto+ * kjs/grammar.y:+ (makeMultNode): Transform as follows:+ +FOO * BAR ==> FOO * BAR+ FOO * +BAR ==> FOO * BAR+ FOO * 1 ==> +FOO+ 1 * FOO ==> +FOO+ (makeDivNode): Transform as follows:+ +FOO / BAR ==> FOO / BAR+ FOO / +BAR ==> FOO / BAR+ (makeSubNode): Transform as follows:+ +FOO - BAR ==> FOO - BAR+ FOO - +BAR ==> FOO - BAR+ * kjs/nodes.h:+ (JSC::ExpressionNode::stripUnaryPlus): Helper for above+ grammar.y changes+ (JSC::UnaryPlusNode::stripUnaryPlus): ditto++2008-10-08 Maciej Stachowiak <mjs@apple.com>++ Reviewed by Oliver Hunt. - correctly handle appending -0 to a string, it should stringify as just 0 diff --git a/JavaScriptCore/VM/CTI.cpp b/JavaScriptCore/VM/CTI.cppindex 0644a6e..29d6537 100644--- a/JavaScriptCore/VM/CTI.cpp+++ b/JavaScriptCore/VM/CTI.cpp@@ -1767,8 +1767,20 @@ void CTI::privateCompileMainPass() break; } case op_to_jsnumber: {- emitGetPutArg(instruction[i + 2].u.operand, 0, X86::ecx);- emitCall(i, Machine::cti_op_to_jsnumber);+ emitGetArg(instruction[i + 2].u.operand, X86::eax);+ + m_jit.testl_i32r(JSImmediate::TagBitTypeInteger, X86::eax);+ X86Assembler::JmpSrc wasImmediate = m_jit.emitUnlinkedJnz();++ emitJumpSlowCaseIfNotJSCell(X86::eax, i);++ m_jit.movl_mr(OBJECT_OFFSET(JSCell, m_structureID), X86::eax, X86::ecx);+ m_jit.cmpl_i32m(NumberType, OBJECT_OFFSET(StructureID, m_typeInfo.m_type), X86::ecx);+ + m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJne(), i));+ + m_jit.link(wasImmediate, m_jit.label());+ emitPutResult(instruction[i + 1].u.operand); i += 3; break;@@ -2518,6 +2530,17 @@ void CTI::privateCompileSlowCases() i += 7; break; }+ case op_to_jsnumber: {+ m_jit.link(iter->from, m_jit.label());+ m_jit.link(iter->from, m_jit.label());++ emitPutArg(X86::eax, 0);+ emitCall(i, Machine::cti_op_to_jsnumber);++ emitPutResult(instruction[i + 1].u.operand);+ i += 3;+ break;+ } default: ASSERT_NOT_REACHED();diff --git a/JavaScriptCore/VM/Machine.cpp b/JavaScriptCore/VM/Machine.cppindex ae35bbe..adac834 100644--- a/JavaScriptCore/VM/Machine.cpp+++ b/JavaScriptCore/VM/Machine.cpp@@ -1815,10 +1815,16 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, RegisterFile* registerFile, */ int dst = (++vPC)->u.operand; int src = (++vPC)->u.operand;- JSValue* result = r[src].jsValue(exec)->toJSNumber(exec);- VM_CHECK_EXCEPTION(); - r[dst] = result;+ JSValue* srcVal = r[src].jsValue(exec);++ if (LIKELY(JSImmediate::isNumber(srcVal) || static_cast<JSCell*>(srcVal)->structureID()->typeInfo().type() == NumberType)) {+ r[dst] = r[src];+ } else {+ JSValue* result = srcVal->toJSNumber(exec);+ VM_CHECK_EXCEPTION();+ r[dst] = result;+ } ++vPC; NEXT_OPCODE;diff --git a/JavaScriptCore/kjs/grammar.y b/JavaScriptCore/kjs/grammar.yindex 81e84b3..d605547 100644--- a/JavaScriptCore/kjs/grammar.y+++ b/JavaScriptCore/kjs/grammar.y@@ -1424,13 +1424,26 @@ static ExpressionNode* makeBitwiseNotNode(void* globalPtr, ExpressionNode* expr) static ExpressionNode* makeMultNode(void* globalPtr, ExpressionNode* expr1, ExpressionNode* expr2, bool rightHasAssignments) {+ expr1 = expr1->stripUnaryPlus();+ expr2 = expr2->stripUnaryPlus();+ if (expr1->isNumber() && expr2->isNumber()) return makeNumberNode(globalPtr, static_cast<NumberNode*>(expr1)->value() * static_cast<NumberNode*>(expr2)->value());++ if (expr1->isNumber() && static_cast<NumberNode*>(expr1)->value() == 1)+ return new UnaryPlusNode(GLOBAL_DATA, expr2);++ if (expr2->isNumber() && static_cast<NumberNode*>(expr2)->value() == 1)+ return new UnaryPlusNode(GLOBAL_DATA, expr1);+ return new MultNode(GLOBAL_DATA, expr1, expr2, rightHasAssignments); } static ExpressionNode* makeDivNode(void* globalPtr, ExpressionNode* expr1, ExpressionNode* expr2, bool rightHasAssignments) {+ expr1 = expr1->stripUnaryPlus();+ expr2 = expr2->stripUnaryPlus();+ if (expr1->isNumber() && expr2->isNumber()) return makeNumberNode(globalPtr, static_cast<NumberNode*>(expr1)->value() / static_cast<NumberNode*>(expr2)->value()); return new DivNode(GLOBAL_DATA, expr1, expr2, rightHasAssignments);@@ -1445,6 +1458,9 @@ static ExpressionNode* makeAddNode(void* globalPtr, ExpressionNode* expr1, Expre static ExpressionNode* makeSubNode(void* globalPtr, ExpressionNode* expr1, ExpressionNode* expr2, bool rightHasAssignments) {+ expr1 = expr1->stripUnaryPlus();+ expr2 = expr2->stripUnaryPlus();+ if (expr1->isNumber() && expr2->isNumber()) return makeNumberNode(globalPtr, static_cast<NumberNode*>(expr1)->value() - static_cast<NumberNode*>(expr2)->value()); return new SubNode(GLOBAL_DATA, expr1, expr2, rightHasAssignments);diff --git a/JavaScriptCore/kjs/nodes.h b/JavaScriptCore/kjs/nodes.hindex 69a055f..181cc0a 100644--- a/JavaScriptCore/kjs/nodes.h+++ b/JavaScriptCore/kjs/nodes.h@@ -226,6 +226,8 @@ namespace JSC { virtual bool isBracketAccessorNode() const JSC_FAST_CALL { return false; } virtual bool isDotAccessorNode() const JSC_FAST_CALL { return false; } + virtual ExpressionNode* stripUnaryPlus() { return this; }+ ResultType resultDescriptor() const JSC_FAST_CALL { return m_resultDesc; } // This needs to be in public in order to compile using GCC 3.x @@ -1179,6 +1181,8 @@ namespace JSC { { } + virtual ExpressionNode* stripUnaryPlus() { return m_expr.get(); }+ virtual OpcodeID opcode() const JSC_FAST_CALL { return op_to_jsnumber; } virtual void streamTo(SourceStream&) const JSC_FAST_CALL; virtual Precedence precedence() const { return PrecUnary; }