JavaScriptCore:
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Oct 2008 23:59:26 +0000 (23:59 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Oct 2008 23:59:26 +0000 (23:59 +0000)
2008-10-09  Gavin Barraclough  <barraclough@apple.com>

        Reviewed by Cameron Zwarich.

        Fix for bug #21160, x=0;1/(x*-1) == -Infinity

        * ChangeLog:
        * VM/CTI.cpp:
        (JSC::CTI::emitFastArithDeTagImmediate):
        (JSC::CTI::emitFastArithDeTagImmediateJumpIfZero):
        (JSC::CTI::compileBinaryArithOp):
        (JSC::CTI::compileBinaryArithOpSlowCase):
        (JSC::CTI::privateCompileMainPass):
        (JSC::CTI::privateCompileSlowCases):
        * VM/CTI.h:
        * masm/X86Assembler.h:
        (JSC::X86Assembler::):
        (JSC::X86Assembler::emitUnlinkedJs):

LayoutTests:

2008-10-09  Gavin Barraclough  <barraclough@apple.com>

        Reviewed by Cameron Zwarich.

        Correct results for -0 cases.

        * fast/js/math-transforms-expected.txt:

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

JavaScriptCore/ChangeLog
JavaScriptCore/VM/CTI.cpp
JavaScriptCore/VM/CTI.h
JavaScriptCore/masm/X86Assembler.h
LayoutTests/ChangeLog
LayoutTests/fast/js/math-transforms-expected.txt

index 7711351c51988edab9dd8f23b93f4bfbd2088350..56343812b4d366abd7e3ca44bc6a17f072fc7f75 100644 (file)
@@ -1,3 +1,22 @@
+2008-10-09  Gavin Barraclough  <barraclough@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        Fix for bug #21160, x=0;1/(x*-1) == -Infinity
+
+        * ChangeLog:
+        * VM/CTI.cpp:
+        (JSC::CTI::emitFastArithDeTagImmediate):
+        (JSC::CTI::emitFastArithDeTagImmediateJumpIfZero):
+        (JSC::CTI::compileBinaryArithOp):
+        (JSC::CTI::compileBinaryArithOpSlowCase):
+        (JSC::CTI::privateCompileMainPass):
+        (JSC::CTI::privateCompileSlowCases):
+        * VM/CTI.h:
+        * masm/X86Assembler.h:
+        (JSC::X86Assembler::):
+        (JSC::X86Assembler::emitUnlinkedJs):
+
 2008-10-09  Cameron Zwarich  <zwarich@apple.com>
 
         Reviewed by Oliver Hunt.
index 576fa9b909617add41ab782f6bcfae6bef8f14fd..78fd0f8307e8c33fff25696a251e888415b5a126 100644 (file)
@@ -414,10 +414,15 @@ ALWAYS_INLINE unsigned CTI::getDeTaggedConstantImmediate(JSValue* imm)
 
 ALWAYS_INLINE void CTI::emitFastArithDeTagImmediate(X86Assembler::RegisterID reg)
 {
-    // op_mod relies on this being a sub - setting zf if result is 0.
     m_jit.subl_i8r(JSImmediate::TagBitTypeInteger, reg);
 }
 
+ALWAYS_INLINE X86Assembler::JmpSrc CTI::emitFastArithDeTagImmediateJumpIfZero(X86Assembler::RegisterID reg)
+{
+    m_jit.subl_i8r(JSImmediate::TagBitTypeInteger, reg);
+    return m_jit.emitUnlinkedJe();
+}
+
 ALWAYS_INLINE void CTI::emitFastArithReTagImmediate(X86Assembler::RegisterID reg)
 {
     m_jit.addl_i8r(JSImmediate::TagBitTypeInteger, reg);
@@ -808,8 +813,19 @@ void CTI::compileBinaryArithOp(OpcodeID opcodeID, unsigned dst, unsigned src1, u
         emitFastArithReTagImmediate(X86::eax);
     } else {
         ASSERT(opcodeID == op_mul);
-        emitFastArithDeTagImmediate(X86::eax);
+        // convert eax & edx from JSImmediates to ints, and check if either are zero
         emitFastArithImmToInt(X86::edx);
+        X86Assembler::JmpSrc op1Zero = emitFastArithDeTagImmediateJumpIfZero(X86::eax);
+        m_jit.testl_rr(X86::edx, X86::edx);
+        X86Assembler::JmpSrc op2NonZero = m_jit.emitUnlinkedJne();
+        m_jit.link(op1Zero, m_jit.label());
+        // if either input is zero, add the two together, and check if the result is < 0.
+        // If it is, we have a problem (N < 0), (N * 0) == -0, not representatble as a JSImmediate. 
+        m_jit.movl_rr(X86::eax, X86::ecx);
+        m_jit.addl_rr(X86::edx, X86::ecx);
+        m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJs(), i));
+        // Skip the above check if neither input is zero
+        m_jit.link(op2NonZero, m_jit.label());
         m_jit.imull_rr(X86::edx, X86::eax);
         m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJo(), i));
         emitFastArithReTagImmediate(X86::eax);
@@ -853,6 +869,10 @@ void CTI::compileBinaryArithOpSlowCase(OpcodeID opcodeID, Vector<SlowCaseEntry>:
     } else
         m_jit.link((++iter)->from, here);
 
+    // additional entry point to handle -0 cases.
+    if (opcodeID == op_mul)
+        m_jit.link((++iter)->from, here);
+
     emitGetPutArg(src1, 0, X86::ecx);
     emitGetPutArg(src2, 4, X86::ecx);
     if (opcodeID == op_add)
@@ -1139,19 +1159,23 @@ void CTI::privateCompileMainPass()
             unsigned src1 = instruction[i + 2].u.operand;
             unsigned src2 = instruction[i + 3].u.operand;
 
-            if (JSValue* src1Value = getConstantImmediateNumericArg(src1)) {
+            // For now, only plant a fast int case if the constant operand is greater than zero.
+            JSValue* src1Value = getConstantImmediateNumericArg(src1);
+            JSValue* src2Value = getConstantImmediateNumericArg(src2);
+            int32_t value;
+            if (src1Value && ((value = JSImmediate::intValue(src1Value)) > 0)) {
                 emitGetArg(src2, X86::eax);
                 emitJumpSlowCaseIfNotImmNum(X86::eax, i);
-                emitFastArithImmToInt(X86::eax);
-                m_jit.imull_i32r(X86::eax, getDeTaggedConstantImmediate(src1Value), X86::eax);
+                emitFastArithDeTagImmediate(X86::eax);
+                m_jit.imull_i32r(X86::eax, value, X86::eax);
                 m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJo(), i));
                 emitFastArithReTagImmediate(X86::eax);
                 emitPutResult(dst);
-            } else if (JSValue* src2Value = getConstantImmediateNumericArg(src2)) {
+            } else if (src2Value && ((value = JSImmediate::intValue(src2Value)) > 0)) {
                 emitGetArg(src1, X86::eax);
                 emitJumpSlowCaseIfNotImmNum(X86::eax, i);
-                emitFastArithImmToInt(X86::eax);
-                m_jit.imull_i32r(X86::eax, getDeTaggedConstantImmediate(src2Value), X86::eax);
+                emitFastArithDeTagImmediate(X86::eax);
+                m_jit.imull_i32r(X86::eax, value, X86::eax);
                 m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJo(), i));
                 emitFastArithReTagImmediate(X86::eax);
                 emitPutResult(dst);
@@ -1627,8 +1651,7 @@ void CTI::privateCompileMainPass()
             emitJumpSlowCaseIfNotImmNum(X86::eax, i);
             emitJumpSlowCaseIfNotImmNum(X86::ecx, i);
             emitFastArithDeTagImmediate(X86::eax);
-            emitFastArithDeTagImmediate(X86::ecx);
-            m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJe(), i)); // This is checking if the last detag resulted in a value 0.
+            m_slowCases.append(SlowCaseEntry(emitFastArithDeTagImmediateJumpIfZero(X86::ecx), i));
             m_jit.cdq();
             m_jit.idivl_r(X86::ecx);
             emitFastArithReTagImmediate(X86::edx);
@@ -2509,8 +2532,19 @@ void CTI::privateCompileSlowCases()
             int dst = instruction[i + 1].u.operand;
             int src1 = instruction[i + 2].u.operand;
             int src2 = instruction[i + 3].u.operand;
-            if (getConstantImmediateNumericArg(src1) || getConstantImmediateNumericArg(src2)) {
+            JSValue* src1Value = getConstantImmediateNumericArg(src1);
+            JSValue* src2Value = getConstantImmediateNumericArg(src2);
+            int32_t value;
+            if (src1Value && ((value = JSImmediate::intValue(src1Value)) > 0)) {
+                m_jit.link(iter->from, m_jit.label());
+                // There is an extra slow case for (op1 * -N) or (-N * op2), to check for 0 since this should produce a result of -0.
+                emitGetPutArg(src1, 0, X86::ecx);
+                emitGetPutArg(src2, 4, X86::ecx);
+                emitCall(i, Machine::cti_op_mul);
+                emitPutResult(dst);
+            } else if (src2Value && ((value = JSImmediate::intValue(src2Value)) > 0)) {
                 m_jit.link(iter->from, m_jit.label());
+                // There is an extra slow case for (op1 * -N) or (-N * op2), to check for 0 since this should produce a result of -0.
                 emitGetPutArg(src1, 0, X86::ecx);
                 emitGetPutArg(src2, 4, X86::ecx);
                 emitCall(i, Machine::cti_op_mul);
index 165b468f52db77090a574c174c1039e2a4852697..f4b9f7258ada46f984154c01851e15ac510ce3ea 100644 (file)
@@ -400,6 +400,7 @@ namespace JSC {
         void emitJumpSlowCaseIfNotImmNums(X86Assembler::RegisterID, X86Assembler::RegisterID, unsigned opcodeIndex);
 
         void emitFastArithDeTagImmediate(X86Assembler::RegisterID);
+        X86Assembler::JmpSrc emitFastArithDeTagImmediateJumpIfZero(X86Assembler::RegisterID);
         void emitFastArithReTagImmediate(X86Assembler::RegisterID);
         void emitFastArithPotentiallyReTagImmediate(X86Assembler::RegisterID);
         void emitFastArithImmToInt(X86Assembler::RegisterID);
index c5c1fc5c9dab04c7e68a046f7e8eb1e8d15dd09b..068b917631061acaa1eed843bf0149d0bad9d12e 100644 (file)
@@ -235,6 +235,7 @@ public:
         OP2_JNE_rel32       = 0x85,
         OP2_JBE_rel32       = 0x86,
         OP2_JA_rel32        = 0x87,
+        OP2_JS_rel32        = 0x88,
         OP2_JP_rel32        = 0x8A,
         OP2_JL_rel32        = 0x8C,
         OP2_JGE_rel32       = 0x8D,
@@ -977,6 +978,14 @@ public:
         return JmpSrc(m_buffer->getOffset());
     }
     
+    JmpSrc emitUnlinkedJs()
+    {
+        m_buffer->putByte(OP_2BYTE_ESCAPE);
+        m_buffer->putByte(OP2_JS_rel32);
+        m_buffer->putInt(0);
+        return JmpSrc(m_buffer->getOffset());
+    }
+    
     void emitPredictionNotTaken()
     {
         m_buffer->putByte(PRE_PREDICT_BRANCH_NOT_TAKEN);
index b370a4e54a67f3c8ae474e534c9ca35368879782..030c6a73f385487f6360743741c842b56c059ee9 100644 (file)
@@ -1,3 +1,11 @@
+2008-10-09  Gavin Barraclough  <barraclough@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        Correct results for -0 cases.
+
+        * fast/js/math-transforms-expected.txt:
+
 2008-10-09  Chris Marrin  <cmarrin@apple.com>
 
         Reviewed by Dan Bernstein.
index d589a6ad1e1dcec140ff35edafbac156f5230fd4..f0dfb930222fba8ec3edeaceee74d9236af527c6 100644 (file)
@@ -714,11 +714,11 @@ PASS values.minusOne - +values.minusOne is 0
 PASS +values.minusOne - +values.minusOne is values.minusOne - values.minusOne
 PASS +values.minusOne - +values.minusOne is 0
 PASS +values.minusOne * values.zero is values.minusOne * values.zero
-PASS +values.minusOne * values.zero is 0
+PASS +values.minusOne * values.zero is -0
 PASS values.minusOne * +values.zero is values.minusOne * values.zero
-PASS values.minusOne * +values.zero is 0
+PASS values.minusOne * +values.zero is -0
 PASS +values.minusOne * +values.zero is values.minusOne * values.zero
-PASS +values.minusOne * +values.zero is 0
+PASS +values.minusOne * +values.zero is -0
 PASS +values.minusOne / values.zero is values.minusOne / values.zero
 PASS +values.minusOne / values.zero is -Infinity
 PASS values.minusOne / +values.zero is values.minusOne / values.zero
@@ -894,11 +894,11 @@ PASS values.zero - +values.one is -1
 PASS +values.zero - +values.one is values.zero - values.one
 PASS +values.zero - +values.one is -1
 PASS +values.zero * values.minusOne is values.zero * values.minusOne
-PASS +values.zero * values.minusOne is 0
+PASS +values.zero * values.minusOne is -0
 PASS values.zero * +values.minusOne is values.zero * values.minusOne
-PASS values.zero * +values.minusOne is 0
+PASS values.zero * +values.minusOne is -0
 PASS +values.zero * +values.minusOne is values.zero * values.minusOne
-PASS +values.zero * +values.minusOne is 0
+PASS +values.zero * +values.minusOne is -0
 PASS +values.zero / values.minusOne is values.zero / values.minusOne
 PASS +values.zero / values.minusOne is -0
 PASS values.zero / +values.minusOne is values.zero / values.minusOne