Changeset 189148 in webkit

Timestamp:

Aug 28, 2015, 11:52:10 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Unreviewed, rolling out r189136.
​https://bugs.webkit.org/show_bug.cgi?id=148608

Made JSC tests flaky (Requested by ap on #webkit).

Reverted changeset:

"[JSC][x86] Improve the compare functions when comparing with
zero"
​https://bugs.webkit.org/show_bug.cgi?id=148536
​http://trac.webkit.org/changeset/189136

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssembler.h (modified) (3 diffs)
• assembler/MacroAssemblerARMv7.h (modified) (4 diffs)
• assembler/MacroAssemblerX86Common.h (modified) (2 diffs)
• dfg/DFGSpeculativeJIT.cpp (modified) (1 diff)
• dfg/DFGSpeculativeJIT64.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-08-28  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r189136.
+        https://bugs.webkit.org/show_bug.cgi?id=148608
+
+        Made JSC tests flaky (Requested by ap on #webkit).
+
+        Reverted changeset:
+
+        "[JSC][x86] Improve the compare functions when comparing with
+        zero"
+        https://bugs.webkit.org/show_bug.cgi?id=148536
+        http://trac.webkit.org/changeset/189136
+
 2015-08-28  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/JavaScriptCore/assembler/MacroAssembler.h

@@ -113 +113 @@
     using MacroAssemblerBase::jump;
     using MacroAssemblerBase::branch32;
-    using MacroAssemblerBase::compare32;
     using MacroAssemblerBase::move;
     using MacroAssemblerBase::add32;
@@ -331 +330 @@
     }
 
-    void compare32(RelationalCondition cond, Imm32 left, RegisterID right, RegisterID dest)
-    {
-        compare32(commute(cond), right, left, dest);
-    }
-
     void branchTestPtr(ResultCondition cond, RegisterID reg, Label target)
     {
@@ -1516 +1510 @@
     }
 
-    void compare32(RelationalCondition cond, RegisterID left, Imm32 right, RegisterID dest)
-    {
-        if (shouldBlind(right)) {
-            if (haveScratchRegisterForBlinding()) {
-                loadXorBlindedConstant(xorBlindConstant(right), scratchRegisterForBlinding());
-                compare32(cond, left, scratchRegisterForBlinding(), dest);
-            }
-            // If we don't have a scratch register available for use, we'll just
-            // place a random number of nops.
-            uint32_t nopCount = random() & 3;
-            while (nopCount--)
-                nop();
-            compare32(cond, left, right.asTrustedImm32(), dest);
-        }
-
-        compare32(cond, left, right.asTrustedImm32(), dest);
-    }
-
     Jump branchAdd32(ResultCondition cond, RegisterID src, Imm32 imm, RegisterID dest)
     {

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

@@ -1306 +1306 @@
 
     // Should we be using TEQ for equal/not-equal?
-    void compare32AndSetFlags(RegisterID left, TrustedImm32 right)
+    void compare32(RegisterID left, TrustedImm32 right)
     {
         int32_t imm = right.m_value;
@@ -1364 +1364 @@
     Jump branch32(RelationalCondition cond, RegisterID left, TrustedImm32 right)
     {
-        compare32AndSetFlags(left, right);
+        compare32(left, right);
         return Jump(makeBranch(cond));
     }
@@ -1422 +1422 @@
     Jump branch8(RelationalCondition cond, RegisterID left, TrustedImm32 right)
     {
-        compare32AndSetFlags(left, right);
+        compare32(left, right);
         return Jump(makeBranch(cond));
     }
@@ -1722 +1722 @@
     void compare32(RelationalCondition cond, RegisterID left, TrustedImm32 right, RegisterID dest)
     {
-        compare32AndSetFlags(left, right);
+        compare32(left, right);
         m_assembler.it(armV7Condition(cond), false);
         m_assembler.mov(dest, ARMThumbImmediate::makeUInt16(1));

trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h

@@ -1125 +1125 @@
     Jump branch32(RelationalCondition cond, RegisterID left, TrustedImm32 right)
     {
-        if (!right.m_value && (cond == Equal || cond == NotEqual || cond == LessThan || cond == GreaterThanOrEqual)) {
-            ResultCondition resultCondition;
-            switch (cond) {
-            case Equal:
-                resultCondition = Zero;
-                break;
-            case NotEqual:
-                resultCondition = NonZero;
-                break;
-            case LessThan:
-                resultCondition = Signed;
-                break;
-            case GreaterThanOrEqual:
-                resultCondition = PositiveOrZero;
-                break;
-            default:
-                RELEASE_ASSERT_NOT_REACHED();
-            }
+        if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
             m_assembler.testl_rr(left, left);
-            return Jump(m_assembler.jCC(x86Condition(resultCondition)));
-        }
-        m_assembler.cmpl_ir(right.m_value, left);
+        else
+            m_assembler.cmpl_ir(right.m_value, left);
         return Jump(m_assembler.jCC(x86Condition(cond)));
     }
@@ -1456 +1438 @@
     void compare32(RelationalCondition cond, RegisterID left, TrustedImm32 right, RegisterID dest)
     {
-        if (!right.m_value && (cond == Equal || cond == NotEqual || cond == LessThan || cond == GreaterThanOrEqual)) {
-            ResultCondition resultCondition;
-            switch (cond) {
-            case Equal:
-                resultCondition = Zero;
-                break;
-            case NotEqual:
-                resultCondition = NonZero;
-                break;
-            case LessThan:
-                resultCondition = Signed;
-                break;
-            case GreaterThanOrEqual:
-                resultCondition = PositiveOrZero;
-                break;
-            default:
-                RELEASE_ASSERT_NOT_REACHED();
-            }
+        if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
             m_assembler.testl_rr(left, left);
-            set32(x86Condition(resultCondition), dest);
-            return;
-        }
-        m_assembler.cmpl_ir(right.m_value, left);
+        else
+            m_assembler.cmpl_ir(right.m_value, left);
         set32(x86Condition(cond), dest);
     }

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -1279 +1279 @@
     }
 
-    if (node->child1()->isInt32Constant()) {
-        int32_t imm = node->child1()->asInt32();
+    if (node->child1()->isBooleanConstant()) {
+        bool imm = node->child1()->asBoolean();
         SpeculateBooleanOperand op2(this, node->child2());
-        branch32(condition, JITCompiler::Imm32(imm), op2.gpr(), taken);
-    } else if (node->child2()->isInt32Constant()) {
+        branch32(condition, JITCompiler::Imm32(static_cast<int32_t>(JSValue::encode(jsBoolean(imm)))), op2.gpr(), taken);
+    } else if (node->child2()->isBooleanConstant()) {
         SpeculateBooleanOperand op1(this, node->child1());
-        int32_t imm = node->child2()->asInt32();
-        branch32(condition, op1.gpr(), JITCompiler::Imm32(imm), taken);
+        bool imm = node->child2()->asBoolean();
+        branch32(condition, op1.gpr(), JITCompiler::Imm32(static_cast<int32_t>(JSValue::encode(jsBoolean(imm)))), taken);
     } else {
         SpeculateBooleanOperand op1(this, node->child1());

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

@@ -1471 +1471 @@
 void SpeculativeJIT::compileInt32Compare(Node* node, MacroAssembler::RelationalCondition condition)
 {
-    if (node->child1()->isInt32Constant()) {
-        SpeculateInt32Operand op2(this, node->child2());
-        GPRTemporary result(this, Reuse, op2);
-        int32_t imm = node->child1()->asInt32();
-        m_jit.compare32(condition, JITCompiler::Imm32(imm), op2.gpr(), result.gpr());
-
-        // If we add a DataFormatBool, we should use it here.
-        m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
-        jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
-    } else if (node->child2()->isInt32Constant()) {
-        SpeculateInt32Operand op1(this, node->child1());
-        GPRTemporary result(this, Reuse, op1);
-        int32_t imm = node->child2()->asInt32();
-        m_jit.compare32(condition, op1.gpr(), JITCompiler::Imm32(imm), result.gpr());
-
-        // If we add a DataFormatBool, we should use it here.
-        m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
-        jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
-    } else {
-        SpeculateInt32Operand op1(this, node->child1());
-        SpeculateInt32Operand op2(this, node->child2());
-        GPRTemporary result(this, Reuse, op1, op2);
-        m_jit.compare32(condition, op1.gpr(), op2.gpr(), result.gpr());
-
-        // If we add a DataFormatBool, we should use it here.
-        m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
-        jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
-    }
+    SpeculateInt32Operand op1(this, node->child1());
+    SpeculateInt32Operand op2(this, node->child2());
+    GPRTemporary result(this, Reuse, op1, op2);
+    
+    m_jit.compare32(condition, op1.gpr(), op2.gpr(), result.gpr());
+    
+    // If we add a DataFormatBool, we should use it here.
+    m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
+    jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
 }