Changeset 197688 in webkit

Timestamp:

Mar 7, 2016 10:30:31 AM (8 years ago)

Author:

benjamin@webkit.org

Message:

[JSC] Simplify the overflow check of ArithAbs
​https://bugs.webkit.org/show_bug.cgi?id=155063

Reviewed by Geoffrey Garen.

The only integer that overflow abs(int32) is INT_MIN.
For some reason, our code testing for that case
was checking the top bit of the result specifically.

The code required a large immediate on x86 and an extra
register on ARM64.

This patch turns the overflow check into a branch on
the sign of the result.

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileArithAbs):

• jit/ThunkGenerators.cpp:

(JSC::absThunkGenerator):

• tests/stress/arith-abs-overflow.js: Added.

(opaqueAbs):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGSpeculativeJIT32_64.cpp (modified) (1 diff)
• dfg/DFGSpeculativeJIT64.cpp (modified) (1 diff)
• ftl/FTLLowerDFGToB3.cpp (modified) (1 diff)
• jit/ThunkGenerators.cpp (modified) (1 diff)
• tests/stress/arith-abs-overflow.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-03-07  Benjamin Poulain  <benjamin@webkit.org>
+
+        [JSC] Simplify the overflow check of ArithAbs
+        https://bugs.webkit.org/show_bug.cgi?id=155063
+
+        Reviewed by Geoffrey Garen.
+
+        The only integer that overflow abs(int32) is INT_MIN.
+        For some reason, our code testing for that case
+        was checking the top bit of the result specifically.
+
+        The code required a large immediate on x86 and an extra
+        register on ARM64.
+
+        This patch turns the overflow check into a branch on
+        the sign of the result.
+
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileArithAbs):
+        * jit/ThunkGenerators.cpp:
+        (JSC::absThunkGenerator):
+        * tests/stress/arith-abs-overflow.js: Added.
+        (opaqueAbs):
+
 2016-03-07  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

@@ -2188 +2188 @@
             m_jit.add32(scratch.gpr(), result.gpr());
             m_jit.xor32(scratch.gpr(), result.gpr());
-            speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branch32(MacroAssembler::Equal, result.gpr(), MacroAssembler::TrustedImm32(1 << 31)));
+            speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Signed, result.gpr()));
             int32Result(result.gpr(), node);
             break;

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

@@ -2336 +2336 @@
             m_jit.xor32(scratch.gpr(), result.gpr());
             if (shouldCheckOverflow(node->arithMode()))
-                speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branch32(MacroAssembler::Equal, result.gpr(), MacroAssembler::TrustedImm32(1 << 31)));
+                speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Signed, result.gpr()));
             int32Result(result.gpr(), node);
             break;

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -1775 +1775 @@
 
             if (shouldCheckOverflow(m_node->arithMode()))
-                speculate(Overflow, noValue(), 0, m_out.equal(result, m_out.constInt32(1 << 31)));
+                speculate(Overflow, noValue(), 0, m_out.lessThan(result, m_out.int32Zero));
 
             setInt32(result);

trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp

@@ -911 +911 @@
     jit.add32(SpecializedThunkJIT::regT1, SpecializedThunkJIT::regT0);
     jit.xor32(SpecializedThunkJIT::regT1, SpecializedThunkJIT::regT0);
-    jit.appendFailure(jit.branch32(MacroAssembler::Equal, SpecializedThunkJIT::regT0, MacroAssembler::TrustedImm32(1 << 31)));
+    jit.appendFailure(jit.branchTest32(MacroAssembler::Signed, SpecializedThunkJIT::regT0));
     jit.returnInt32(SpecializedThunkJIT::regT0);
     nonIntJump.link(&jit);