2008-09-14 Cameron Zwarich <cwzwarich@uwaterloo.ca>
authorcwzwarich@webkit.org <cwzwarich@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Sep 2008 00:26:08 +0000 (00:26 +0000)
committercwzwarich@webkit.org <cwzwarich@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Sep 2008 00:26:08 +0000 (00:26 +0000)
        Reviewed by Maciej Stachowiak.

        Bug 20827: the 'typeof' operator is slow
        <https://bugs.webkit.org/show_bug.cgi?id=20827>

        Optimize the 'typeof' operator when its result is compared to a constant
        string.

        This is a 5.5% speedup on the V8 Earley-Boyer test.

        JavaScriptCore:

        * VM/CTI.cpp:
        (JSC::CTI::privateCompileMainPass):
        * VM/CodeBlock.cpp:
        (JSC::CodeBlock::dump):
        * VM/CodeGenerator.cpp:
        (JSC::CodeGenerator::emitEqualityOp):
        * VM/CodeGenerator.h:
        * VM/Machine.cpp:
        (JSC::jsIsObjectType):
        (JSC::jsIsFunctionType):
        (JSC::Machine::privateExecute):
        (JSC::Machine::cti_op_is_undefined):
        (JSC::Machine::cti_op_is_boolean):
        (JSC::Machine::cti_op_is_number):
        (JSC::Machine::cti_op_is_string):
        (JSC::Machine::cti_op_is_object):
        (JSC::Machine::cti_op_is_function):
        * VM/Machine.h:
        * VM/Opcode.h:
        * kjs/nodes.cpp:
        (JSC::BinaryOpNode::emitCode):
        (JSC::EqualNode::emitCode):
        (JSC::StrictEqualNode::emitCode):
        * kjs/nodes.h:

        LayoutTests:

        * fast/js/resources/typeof-codegen-crash.js: Added.
        * fast/js/typeof-codegen-crash-expected.txt: Added.
        * fast/js/typeof-codegen-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@36412 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
JavaScriptCore/ChangeLog
JavaScriptCore/VM/CTI.cpp
JavaScriptCore/VM/CodeBlock.cpp
JavaScriptCore/VM/CodeGenerator.cpp
JavaScriptCore/VM/CodeGenerator.h
JavaScriptCore/VM/Machine.cpp
JavaScriptCore/VM/Machine.h
JavaScriptCore/VM/Opcode.h
JavaScriptCore/kjs/nodes.cpp
JavaScriptCore/kjs/nodes.h
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/typeof-codegen-crash.js [new file with mode: 0644]
LayoutTests/fast/js/typeof-codegen-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/typeof-codegen-crash.html [new file with mode: 0644]

index 3b303e2..3ccfe37 100644 (file)
@@ -1,3 +1,40 @@
+2008-09-14  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Maciej Stachowiak.
+
+        Bug 20827: the 'typeof' operator is slow
+        <https://bugs.webkit.org/show_bug.cgi?id=20827>
+
+        Optimize the 'typeof' operator when its result is compared to a constant
+        string.
+
+        This is a 5.5% speedup on the V8 Earley-Boyer test.
+
+        * VM/CTI.cpp:
+        (JSC::CTI::privateCompileMainPass):
+        * VM/CodeBlock.cpp:
+        (JSC::CodeBlock::dump):
+        * VM/CodeGenerator.cpp:
+        (JSC::CodeGenerator::emitEqualityOp):
+        * VM/CodeGenerator.h:
+        * VM/Machine.cpp:
+        (JSC::jsIsObjectType):
+        (JSC::jsIsFunctionType):
+        (JSC::Machine::privateExecute):
+        (JSC::Machine::cti_op_is_undefined):
+        (JSC::Machine::cti_op_is_boolean):
+        (JSC::Machine::cti_op_is_number):
+        (JSC::Machine::cti_op_is_string):
+        (JSC::Machine::cti_op_is_object):
+        (JSC::Machine::cti_op_is_function):
+        * VM/Machine.h:
+        * VM/Opcode.h:
+        * kjs/nodes.cpp:
+        (JSC::BinaryOpNode::emitCode):
+        (JSC::EqualNode::emitCode):
+        (JSC::StrictEqualNode::emitCode):
+        * kjs/nodes.h:
+
 2008-09-14  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Cameron Zwarich.
index d8f738f..ad00c5d 100644 (file)
@@ -374,6 +374,15 @@ CTI::CTI(Machine* machine, ExecState* exec, CodeBlock* codeBlock)
         break; \
     }
 
+#define CTI_COMPILE_UNARY_OP(name) \
+    case name: { \
+        emitGetPutArg(instruction[i + 2].u.operand, 0, X86::ecx); \
+        emitCall(i, Machine::cti_##name); \
+        emitPutResult(instruction[i + 1].u.operand); \
+        i += 3; \
+        break; \
+    }
+
 #if ENABLE(SAMPLING_TOOL)
 OpcodeID currentOpcodeID = static_cast<OpcodeID>(-1);
 #endif
@@ -1150,13 +1159,13 @@ void CTI::privateCompileMainPass()
             i += 1;
             break;
         }
-        case op_typeof: {
-            emitGetPutArg(instruction[i + 2].u.operand, 0, X86::ecx);
-            emitCall(i, Machine::cti_op_typeof);
-            emitPutResult(instruction[i + 1].u.operand);
-            i += 3;
-            break;
-        }
+        CTI_COMPILE_UNARY_OP(op_typeof)
+        CTI_COMPILE_UNARY_OP(op_is_undefined)
+        CTI_COMPILE_UNARY_OP(op_is_boolean)
+        CTI_COMPILE_UNARY_OP(op_is_number)
+        CTI_COMPILE_UNARY_OP(op_is_string)
+        CTI_COMPILE_UNARY_OP(op_is_object)
+        CTI_COMPILE_UNARY_OP(op_is_function)
         CTI_COMPILE_BINARY_OP(op_stricteq)
         CTI_COMPILE_BINARY_OP(op_nstricteq)
         case op_to_jsnumber: {
index 70d77c8..3a20561 100644 (file)
@@ -495,6 +495,30 @@ void CodeBlock::dump(ExecState* exec, const Vector<Instruction>::const_iterator&
             printUnaryOp(location, it, "typeof");
             break;
         }
+        case op_is_undefined: {
+            printUnaryOp(location, it, "is_undefined");
+            break;
+        }
+        case op_is_boolean: {
+            printUnaryOp(location, it, "is_boolean");
+            break;
+        }
+        case op_is_number: {
+            printUnaryOp(location, it, "is_number");
+            break;
+        }
+        case op_is_string: {
+            printUnaryOp(location, it, "is_string");
+            break;
+        }
+        case op_is_object: {
+            printUnaryOp(location, it, "is_object");
+            break;
+        }
+        case op_is_function: {
+            printUnaryOp(location, it, "is_function");
+            break;
+        }
         case op_in: {
             printBinaryOp(location, it, "in");
             break;
index 40c0e13..00e7b6e 100644 (file)
@@ -658,6 +658,71 @@ RegisterID* CodeGenerator::emitBinaryOp(OpcodeID opcode, RegisterID* dst, Regist
     return dst;
 }
 
+RegisterID* CodeGenerator::emitEqualityOp(OpcodeID opcode, RegisterID* dst, RegisterID* src1, RegisterID* src2)
+{
+    if (m_lastOpcodeID == op_typeof) {
+        int dstIndex;
+        int srcIndex;
+
+        retrieveLastUnaryOp(dstIndex, srcIndex);
+
+        if (src1->index() == dstIndex
+            && src1->isTemporary()
+            && static_cast<unsigned>(src2->index()) < m_codeBlock->constantRegisters.size()
+            && m_codeBlock->constantRegisters[src2->index()].jsValue(globalExec())->isString()) {
+            const UString& value = static_cast<JSString*>(m_codeBlock->constantRegisters[src2->index()].jsValue(globalExec()))->value();
+            if (value == "undefined") {
+                rewindUnaryOp();
+                emitOpcode(op_is_undefined);
+                instructions().append(dst->index());
+                instructions().append(srcIndex);
+                return dst;
+            }
+            if (value == "boolean") {
+                rewindUnaryOp();
+                emitOpcode(op_is_boolean);
+                instructions().append(dst->index());
+                instructions().append(srcIndex);
+                return dst;
+            }
+            if (value == "number") {
+                rewindUnaryOp();
+                emitOpcode(op_is_number);
+                instructions().append(dst->index());
+                instructions().append(srcIndex);
+                return dst;
+            }
+            if (value == "string") {
+                rewindUnaryOp();
+                emitOpcode(op_is_string);
+                instructions().append(dst->index());
+                instructions().append(srcIndex);
+                return dst;
+            }
+            if (value == "object") {
+                rewindUnaryOp();
+                emitOpcode(op_is_object);
+                instructions().append(dst->index());
+                instructions().append(srcIndex);
+                return dst;
+            }
+            if (value == "function") {
+                rewindUnaryOp();
+                emitOpcode(op_is_function);
+                instructions().append(dst->index());
+                instructions().append(srcIndex);
+                return dst;
+            }
+        }
+    }
+
+    emitOpcode(opcode);
+    instructions().append(dst->index());
+    instructions().append(src1->index());
+    instructions().append(src2->index());
+    return dst;
+}
+
 RegisterID* CodeGenerator::emitLoad(RegisterID* dst, bool b)
 {
     return emitLoad(dst, jsBoolean(b));
index 60e17bf..f4fdafa 100644 (file)
@@ -234,6 +234,7 @@ namespace JSC {
 
         RegisterID* emitUnaryOp(OpcodeID, RegisterID* dst, RegisterID* src);
         RegisterID* emitBinaryOp(OpcodeID, RegisterID* dst, RegisterID* src1, RegisterID* src2);
+        RegisterID* emitEqualityOp(OpcodeID, RegisterID* dst, RegisterID* src1, RegisterID* src2);
         RegisterID* emitUnaryNoDstOp(OpcodeID, RegisterID* src);
 
         RegisterID* emitNewObject(RegisterID* dst);
index cd8df59..ee72e3a 100644 (file)
@@ -247,6 +247,34 @@ static JSValue* jsTypeStringForValue(ExecState* exec, JSValue* v)
     return jsNontrivialString(exec, "object");
 }
 
+static bool jsIsObjectType(JSValue* v)
+{
+    if (JSImmediate::isImmediate(v))
+        return v->isNull();
+
+    JSType type = static_cast<JSCell*>(v)->structureID()->type();
+    if (type == NumberType || type == StringType)
+        return false;
+    if (type == ObjectType) {
+        if (static_cast<JSObject*>(v)->masqueradeAsUndefined())
+            return false;
+        CallData callData;
+        if (static_cast<JSObject*>(v)->getCallData(callData) != CallTypeNone)
+            return false;
+    }
+    return true;
+}
+
+static bool jsIsFunctionType(JSValue* v)
+{
+    if (v->isObject()) {
+        CallData callData;
+        if (static_cast<JSObject*>(v)->getCallData(callData) != CallTypeNone)
+            return true;
+    }
+    return false;
+}
+
 static bool NEVER_INLINE resolve(ExecState* exec, Instruction* vPC, Register* r, ScopeChainNode* scopeChain, CodeBlock* codeBlock, JSValue*& exceptionValue)
 {
     int dst = (vPC + 1)->u.operand;
@@ -2045,6 +2073,91 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, ExecState* exec, RegisterFi
         ++vPC;
         NEXT_OPCODE;
     }
+    BEGIN_OPCODE(op_is_undefined) {
+        /* is_undefined dst(r) src(r)
+
+           Determines whether the type string for src according to
+           the ECMAScript rules is "undefined", and puts the result
+           in register dst.
+        */
+        int dst = (++vPC)->u.operand;
+        int src = (++vPC)->u.operand;
+        JSValue* v = r[src].jsValue(exec);
+        r[dst] = jsBoolean(v->isUndefined() || (v->isObject() && static_cast<JSObject*>(v)->masqueradeAsUndefined()));
+
+        ++vPC;
+        NEXT_OPCODE;
+    }
+    BEGIN_OPCODE(op_is_boolean) {
+        /* is_boolean dst(r) src(r)
+
+           Determines whether the type string for src according to
+           the ECMAScript rules is "boolean", and puts the result
+           in register dst.
+        */
+        int dst = (++vPC)->u.operand;
+        int src = (++vPC)->u.operand;
+        r[dst] = jsBoolean(r[src].jsValue(exec)->isBoolean());
+
+        ++vPC;
+        NEXT_OPCODE;
+    }
+    BEGIN_OPCODE(op_is_number) {
+        /* is_number dst(r) src(r)
+
+           Determines whether the type string for src according to
+           the ECMAScript rules is "number", and puts the result
+           in register dst.
+        */
+        int dst = (++vPC)->u.operand;
+        int src = (++vPC)->u.operand;
+        r[dst] = jsBoolean(r[src].jsValue(exec)->isNumber());
+
+        ++vPC;
+        NEXT_OPCODE;
+    }
+    BEGIN_OPCODE(op_is_string) {
+        /* is_string dst(r) src(r)
+
+           Determines whether the type string for src according to
+           the ECMAScript rules is "string", and puts the result
+           in register dst.
+        */
+        int dst = (++vPC)->u.operand;
+        int src = (++vPC)->u.operand;
+        r[dst] = jsBoolean(r[src].jsValue(exec)->isString());
+
+        ++vPC;
+        NEXT_OPCODE;
+    }
+    BEGIN_OPCODE(op_is_object) {
+        /* is_object dst(r) src(r)
+
+           Determines whether the type string for src according to
+           the ECMAScript rules is "object", and puts the result
+           in register dst.
+        */
+        int dst = (++vPC)->u.operand;
+        int src = (++vPC)->u.operand;
+        r[dst] = jsBoolean(jsIsObjectType(r[src].jsValue(exec)));
+
+        ++vPC;
+        NEXT_OPCODE;
+    }
+    BEGIN_OPCODE(op_is_function) {
+        /* is_function dst(r) src(r)
+
+           Determines whether the type string for src according to
+           the ECMAScript rules is "function", and puts the result
+           in register dst.
+        */
+        int dst = (++vPC)->u.operand;
+        int src = (++vPC)->u.operand;
+        r[dst] = jsBoolean(jsIsFunctionType(r[src].jsValue(exec)));
+
+        ++vPC;
+        NEXT_OPCODE;
+    }
     BEGIN_OPCODE(op_in) {
         /* in dst(r) property(r) base(r)
 
@@ -4985,6 +5098,37 @@ JSValue* Machine::cti_op_typeof(CTI_ARGS)
     return jsTypeStringForValue(ARG_exec, ARG_src1);
 }
 
+JSValue* Machine::cti_op_is_undefined(CTI_ARGS)
+{
+    JSValue* v = ARG_src1;
+    return jsBoolean(v->isUndefined() || (v->isObject() && static_cast<JSObject*>(v)->masqueradeAsUndefined()));
+}
+
+JSValue* Machine::cti_op_is_boolean(CTI_ARGS)
+{
+    return jsBoolean(ARG_src1->isBoolean());
+}
+
+JSValue* Machine::cti_op_is_number(CTI_ARGS)
+{
+    return jsBoolean(ARG_src1->isNumber());
+}
+
+JSValue* Machine::cti_op_is_string(CTI_ARGS)
+{
+    return jsBoolean(ARG_src1->isString());
+}
+
+JSValue* Machine::cti_op_is_object(CTI_ARGS)
+{
+    return jsBoolean(jsIsObjectType(ARG_src1));
+}
+
+JSValue* Machine::cti_op_is_function(CTI_ARGS)
+{
+    return jsBoolean(jsIsFunctionType(ARG_src1));
+}
+
 JSValue* Machine::cti_op_stricteq(CTI_ARGS)
 {
     JSValue* src1 = ARG_src1;
index 3f7b490..0519fd4 100644 (file)
@@ -202,6 +202,12 @@ namespace JSC {
         static void SFX_CALL cti_op_push_scope(CTI_ARGS);
         static void SFX_CALL cti_op_pop_scope(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_typeof(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_is_undefined(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_is_boolean(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_is_number(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_is_string(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_is_object(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_is_function(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_stricteq(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_nstricteq(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_to_jsnumber(CTI_ARGS);
index a112671..5792e4a 100644 (file)
@@ -78,6 +78,12 @@ namespace JSC {
         \
         macro(op_instanceof) \
         macro(op_typeof) \
+        macro(op_is_undefined) \
+        macro(op_is_boolean) \
+        macro(op_is_number) \
+        macro(op_is_string) \
+        macro(op_is_object) \
+        macro(op_is_function) \
         macro(op_in) \
         \
         macro(op_resolve) \
index 794ebe1..510d668 100644 (file)
@@ -748,10 +748,10 @@ RegisterID* UnaryOpNode::emitCode(CodeGenerator& generator, RegisterID* dst)
 RegisterID* BinaryOpNode::emitCode(CodeGenerator& generator, RegisterID* dst)
 {
     OpcodeID opcode = this->opcode();
-    if (opcode == op_eq || opcode == op_neq) {
+    if (opcode == op_neq) {
         if (m_expr1->isNull() || m_expr2->isNull()) {
             RefPtr<RegisterID> src = generator.emitNode(dst, m_expr1->isNull() ? m_expr2.get() : m_expr1.get());
-            return generator.emitUnaryOp(opcode == op_eq ? op_eq_null : op_neq_null, generator.finalDestination(dst, src.get()), src.get());
+            return generator.emitUnaryOp(op_neq_null, generator.finalDestination(dst, src.get()), src.get());
         }
     }
 
@@ -760,6 +760,25 @@ RegisterID* BinaryOpNode::emitCode(CodeGenerator& generator, RegisterID* dst)
     return generator.emitBinaryOp(opcode, generator.finalDestination(dst, src1.get()), src1.get(), src2);
 }
 
+RegisterID* EqualNode::emitCode(CodeGenerator& generator, RegisterID* dst)
+{
+    if (m_expr1->isNull() || m_expr2->isNull()) {
+        RefPtr<RegisterID> src = generator.emitNode(dst, m_expr1->isNull() ? m_expr2.get() : m_expr1.get());
+        return generator.emitUnaryOp(op_eq_null, generator.finalDestination(dst, src.get()), src.get());
+    }
+
+    RefPtr<RegisterID> src1 = generator.emitNodeForLeftHandSide(m_expr1.get(), m_rightHasAssignments, m_expr2->isPure(generator));
+    RegisterID* src2 = generator.emitNode(m_expr2.get());
+    return generator.emitEqualityOp(op_eq, generator.finalDestination(dst, src1.get()), src1.get(), src2);
+}
+
+RegisterID* StrictEqualNode::emitCode(CodeGenerator& generator, RegisterID* dst)
+{
+    RefPtr<RegisterID> src1 = generator.emitNodeForLeftHandSide(m_expr1.get(), m_rightHasAssignments, m_expr2->isPure(generator));
+    RegisterID* src2 = generator.emitNode(m_expr2.get());
+    return generator.emitEqualityOp(op_stricteq, generator.finalDestination(dst, src1.get()), src1.get(), src2);
+}
+
 RegisterID* ReverseBinaryOpNode::emitCode(CodeGenerator& generator, RegisterID* dst)
 {
     RefPtr<RegisterID> src1 = generator.emitNodeForLeftHandSide(m_expr1.get(), m_rightHasAssignments, m_expr2->isPure(generator));
index f75b763..39b296a 100644 (file)
@@ -1448,6 +1448,7 @@ namespace JSC {
         {
         }
 
+        virtual RegisterID* emitCode(CodeGenerator&, RegisterID* = 0) JSC_FAST_CALL;
         virtual OpcodeID opcode() const JSC_FAST_CALL { return op_eq; }
         virtual void streamTo(SourceStream&) const JSC_FAST_CALL;
         virtual Precedence precedence() const { return PrecEquality; }
@@ -1472,6 +1473,7 @@ namespace JSC {
         {
         }
 
+        virtual RegisterID* emitCode(CodeGenerator&, RegisterID* = 0) JSC_FAST_CALL;
         virtual OpcodeID opcode() const JSC_FAST_CALL { return op_stricteq; }
         virtual void streamTo(SourceStream&) const JSC_FAST_CALL;
         virtual Precedence precedence() const { return PrecEquality; }
index 0c57742..f912119 100644 (file)
@@ -1,3 +1,15 @@
+2008-09-14  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Maciej Stachowiak.
+
+        Test for a bug in a preliminary version of the patch for
+        Bug 20827: the 'typeof' operator is slow
+        <https://bugs.webkit.org/show_bug.cgi?id=20827>
+
+        * fast/js/resources/typeof-codegen-crash.js: Added.
+        * fast/js/typeof-codegen-crash-expected.txt: Added.
+        * fast/js/typeof-codegen-crash.html: Added.
+
 2008-09-13  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Sam Weinig.
diff --git a/LayoutTests/fast/js/resources/typeof-codegen-crash.js b/LayoutTests/fast/js/resources/typeof-codegen-crash.js
new file mode 100644 (file)
index 0000000..4cdc169
--- /dev/null
@@ -0,0 +1,15 @@
+description(
+"This test for a crash when optimizing expressions of the form 'typeof o == constant' where 'constant' is not a string."
+);
+
+var o = { };
+
+shouldBeFalse("typeof o == undefined");
+shouldBeFalse("typeof o == null");
+shouldBeFalse("typeof o == true");
+shouldBeFalse("typeof o == false");
+shouldBeFalse("typeof o == 1");
+shouldBeFalse("typeof o == 1.0");
+shouldBeFalse("typeof o == { }");
+
+successfullyParsed = true;
diff --git a/LayoutTests/fast/js/typeof-codegen-crash-expected.txt b/LayoutTests/fast/js/typeof-codegen-crash-expected.txt
new file mode 100644 (file)
index 0000000..72fad0f
--- /dev/null
@@ -0,0 +1,16 @@
+This test for a crash when optimizing expressions of the form 'typeof o == constant' where 'constant' is not a string.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS typeof o == undefined is false
+PASS typeof o == null is false
+PASS typeof o == true is false
+PASS typeof o == false is false
+PASS typeof o == 1 is false
+PASS typeof o == 1.0 is false
+PASS typeof o == { } is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/typeof-codegen-crash.html b/LayoutTests/fast/js/typeof-codegen-crash.html
new file mode 100644 (file)
index 0000000..09d7719
--- /dev/null
@@ -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="resources/typeof-codegen-crash.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>