- Revision
- 197688
- Author
- benja...@webkit.org
- Date
- 2016-03-07 10:30:31 -0800 (Mon, 07 Mar 2016)
Log 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):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (197687 => 197688)
--- trunk/Source/_javascript_Core/ChangeLog 2016-03-07 18:25:55 UTC (rev 197687)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-03-07 18:30:31 UTC (rev 197688)
@@ -1,3 +1,31 @@
+2016-03-07 Benjamin Poulain <benja...@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 <bpoul...@apple.com>
[JSC] Improve how DFG zero Floating Point registers
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (197687 => 197688)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-03-07 18:25:55 UTC (rev 197687)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-03-07 18:30:31 UTC (rev 197688)
@@ -2187,7 +2187,7 @@
m_jit.rshift32(result.gpr(), MacroAssembler::TrustedImm32(31), scratch.gpr());
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;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (197687 => 197688)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-03-07 18:25:55 UTC (rev 197687)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-03-07 18:30:31 UTC (rev 197688)
@@ -2335,7 +2335,7 @@
m_jit.add32(scratch.gpr(), result.gpr());
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;
}
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (197687 => 197688)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-03-07 18:25:55 UTC (rev 197687)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-03-07 18:30:31 UTC (rev 197688)
@@ -1774,7 +1774,7 @@
LValue result = m_out.bitXor(mask, m_out.add(mask, value));
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);
break;
Modified: trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp (197687 => 197688)
--- trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp 2016-03-07 18:25:55 UTC (rev 197687)
+++ trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp 2016-03-07 18:30:31 UTC (rev 197688)
@@ -910,7 +910,7 @@
jit.rshift32(SpecializedThunkJIT::regT0, MacroAssembler::TrustedImm32(31), SpecializedThunkJIT::regT1);
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);
// Shame about the double int conversion here.
Added: trunk/Source/_javascript_Core/tests/stress/arith-abs-overflow.js (0 => 197688)
--- trunk/Source/_javascript_Core/tests/stress/arith-abs-overflow.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/arith-abs-overflow.js 2016-03-07 18:30:31 UTC (rev 197688)
@@ -0,0 +1,22 @@
+function opaqueAbs(value)
+{
+ return Math.abs(value);
+}
+noInline(opaqueAbs);
+
+// Warmup.
+for (let i = 0; i < 1e4; ++i) {
+ var positiveResult = opaqueAbs(i);
+ if (positiveResult !== i)
+ throw "Incorrect positive result at i = " + i + " result = " + positiveResult;
+ var negativeResult = opaqueAbs(-i);
+ if (negativeResult !== i)
+ throw "Incorrect negative result at -i = " + -i + " result = " + negativeResult;
+}
+
+// Overflow.
+for (let i = 0; i < 1e4; ++i) {
+ var overflowResult = opaqueAbs(-2147483648);
+ if (overflowResult !== 2147483648)
+ throw "Incorrect overflow result at i = " + i + " result = " + overflowResult;
+}
