[JSC] Integer Multiply of a number by itself does not need negative zero support

https://bugs.webkit.org/show_bug.cgi?id=156895

Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-04-22
Reviewed by Saam Barati.

You cannot produce negative zero by squaring an integer.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileArithMul):
Minor codegen fixes:
-Use the right form of multiply for ARM.
-Use a sign-extended 32bit immediates, that's the one with fast forms
 in the MacroAssembler.

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 52d026c..055a9bf 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,21 @@
+2016-04-22  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Integer Multiply of a number by itself does not need negative zero support
+        https://bugs.webkit.org/show_bug.cgi?id=156895
+
+        Reviewed by Saam Barati.
+
+        You cannot produce negative zero by squaring an integer.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileArithMul):
+        Minor codegen fixes:
+        -Use the right form of multiply for ARM.
+        -Use a sign-extended 32bit immediates, that's the one with fast forms
+         in the MacroAssembler.
+
 2016-04-21  Darin Adler  <darin@apple.com>
 
         Follow-on to the build fix.

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
index 1cea7ac..6586c29 100644 (file)
--- a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
@@ -322,6 +322,11 @@ public:
         m_assembler.smull(dest, dataTempRegister, dest, src);
     }
 
+    void mul32(RegisterID left, RegisterID right, RegisterID dest)
+    {
+        m_assembler.smull(dest, dataTempRegister, left, right);
+    }
+
     void mul32(TrustedImm32 imm, RegisterID src, RegisterID dest)
     {
         move(imm, dataTempRegister);

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 870e5d5..28bf040 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -248,7 +248,8 @@ private:
                 fixIntOrBooleanEdge(rightChild);
                 if (bytecodeCanTruncateInteger(node->arithNodeFlags()))
                     node->setArithMode(Arith::Unchecked);
-                else if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags()))
+                else if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags())
+                    || leftChild.node() == rightChild.node())
                     node->setArithMode(Arith::CheckOverflow);
                 else
                     node->setArithMode(Arith::CheckOverflowAndNegativeZero);
@@ -257,7 +258,8 @@ private:
             if (m_graph.binaryArithShouldSpeculateMachineInt(node, FixupPass)) {
                 fixEdge<Int52RepUse>(leftChild);
                 fixEdge<Int52RepUse>(rightChild);
-                if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags()))
+                if (bytecodeCanIgnoreNegativeZero(node->arithNodeFlags())
+                    || leftChild.node() == rightChild.node())
                     node->setArithMode(Arith::CheckOverflow);
                 else
                     node->setArithMode(Arith::CheckOverflowAndNegativeZero);

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 25cb9e0..4b304e0 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -3964,10 +3964,9 @@ void SpeculativeJIT::compileArithMul(Node* node)
         // We can perform truncated multiplications if we get to this point, because if the
         // fixup phase could not prove that it would be safe, it would have turned us into
         // a double multiplication.
-        if (!shouldCheckOverflow(node->arithMode())) {
-            m_jit.move(reg1, result.gpr());
-            m_jit.mul32(reg2, result.gpr());
-        } else {
+        if (!shouldCheckOverflow(node->arithMode()))
+            m_jit.mul32(reg1, reg2, result.gpr());
+        else {
             speculationCheck(
                 Overflow, JSValueRegs(), 0,
                 m_jit.branchMul32(MacroAssembler::Overflow, reg1, reg2, result.gpr()));
@@ -4031,10 +4030,10 @@ void SpeculativeJIT::compileArithMul(Node* node)
                 MacroAssembler::NonZero, resultGPR);
             speculationCheck(
                 NegativeZero, JSValueRegs(), 0,
-                m_jit.branch64(MacroAssembler::LessThan, op1GPR, TrustedImm64(0)));
+                m_jit.branch64(MacroAssembler::LessThan, op1GPR, TrustedImm32(0)));
             speculationCheck(
                 NegativeZero, JSValueRegs(), 0,
-                m_jit.branch64(MacroAssembler::LessThan, op2GPR, TrustedImm64(0)));
+                m_jit.branch64(MacroAssembler::LessThan, op2GPR, TrustedImm32(0)));
             resultNonZero.link(&m_jit);
         }