- Revision
- 205283
- Author
- sbar...@apple.com
- Date
- 2016-09-01 01:22:21 -0700 (Thu, 01 Sep 2016)
Log Message
JITMathIC was misusing maxJumpReplacementSize
https://bugs.webkit.org/show_bug.cgi?id=161356
<rdar://problem/28065560>
Reviewed by Benjamin Poulain.
JITMathIC was assuming that maxJumpReplacementSize is the size
you'd get if you emitted a patchableJump() using the macro assembler.
This is not true, however. It happens to be true on arm64, x86 and x86-64,
however, it is not true on armv7. This patch introduces an alternative to
maxJumpReplacementSize called patchableJumpSize, and switches JITMathIC
to use that number instead.
* assembler/ARM64Assembler.h:
(JSC::ARM64Assembler::patchableJumpSize):
(JSC::ARM64Assembler::maxJumpReplacementSize): Deleted.
* assembler/ARMv7Assembler.h:
(JSC::ARMv7Assembler::patchableJumpSize):
(JSC::ARMv7Assembler::maxJumpReplacementSize): Deleted.
* assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::patchableJumpSize):
* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::patchableJumpSize):
* assembler/MacroAssemblerX86Common.h:
(JSC::MacroAssemblerX86Common::patchableJumpSize):
* assembler/X86Assembler.h:
(JSC::X86Assembler::patchableJumpSize):
(JSC::X86Assembler::maxJumpReplacementSize): Deleted.
* jit/JITMathIC.h:
(JSC::JITMathIC::generateInline):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (205282 => 205283)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-09-01 08:22:21 UTC (rev 205283)
@@ -1,3 +1,36 @@
+2016-09-01 Saam Barati <sbar...@apple.com>
+
+ JITMathIC was misusing maxJumpReplacementSize
+ https://bugs.webkit.org/show_bug.cgi?id=161356
+ <rdar://problem/28065560>
+
+ Reviewed by Benjamin Poulain.
+
+ JITMathIC was assuming that maxJumpReplacementSize is the size
+ you'd get if you emitted a patchableJump() using the macro assembler.
+ This is not true, however. It happens to be true on arm64, x86 and x86-64,
+ however, it is not true on armv7. This patch introduces an alternative to
+ maxJumpReplacementSize called patchableJumpSize, and switches JITMathIC
+ to use that number instead.
+
+ * assembler/ARM64Assembler.h:
+ (JSC::ARM64Assembler::patchableJumpSize):
+ (JSC::ARM64Assembler::maxJumpReplacementSize): Deleted.
+ * assembler/ARMv7Assembler.h:
+ (JSC::ARMv7Assembler::patchableJumpSize):
+ (JSC::ARMv7Assembler::maxJumpReplacementSize): Deleted.
+ * assembler/MacroAssemblerARM64.h:
+ (JSC::MacroAssemblerARM64::patchableJumpSize):
+ * assembler/MacroAssemblerARMv7.h:
+ (JSC::MacroAssemblerARMv7::patchableJumpSize):
+ * assembler/MacroAssemblerX86Common.h:
+ (JSC::MacroAssemblerX86Common::patchableJumpSize):
+ * assembler/X86Assembler.h:
+ (JSC::X86Assembler::patchableJumpSize):
+ (JSC::X86Assembler::maxJumpReplacementSize): Deleted.
+ * jit/JITMathIC.h:
+ (JSC::JITMathIC::generateInline):
+
2016-08-31 Yusuke Suzuki <utatane....@gmail.com>
[JSC] Add initiator parameter to module pipeline
Modified: trunk/Source/_javascript_Core/assembler/ARM64Assembler.h (205282 => 205283)
--- trunk/Source/_javascript_Core/assembler/ARM64Assembler.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/assembler/ARM64Assembler.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -2512,6 +2512,11 @@
{
return 4;
}
+
+ static constexpr ptrdiff_t patchableJumpSize()
+ {
+ return 4;
+ }
static void replaceWithLoad(void* where)
{
Modified: trunk/Source/_javascript_Core/assembler/ARMv7Assembler.h (205282 => 205283)
--- trunk/Source/_javascript_Core/assembler/ARMv7Assembler.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/assembler/ARMv7Assembler.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -2340,6 +2340,11 @@
return 4;
#endif
}
+
+ static constexpr ptrdiff_t patchableJumpSize()
+ {
+ return 10;
+ }
static void replaceWithLoad(void* instructionStart)
{
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h (205282 => 205283)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -3249,6 +3249,11 @@
return ARM64Assembler::maxJumpReplacementSize();
}
+ static ptrdiff_t patchableJumpSize()
+ {
+ return ARM64Assembler::patchableJumpSize();
+ }
+
RegisterID scratchRegisterForBlinding()
{
// We *do not* have a scratch register for blinding.
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h (205282 => 205283)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -1350,6 +1350,11 @@
return ARMv7Assembler::maxJumpReplacementSize();
}
+ static ptrdiff_t patchableJumpSize()
+ {
+ return ARMv7Assembler::patchableJumpSize();
+ }
+
// Forwards / external control flow operations:
//
// This set of jump and conditional branch operations return a Jump
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h (205282 => 205283)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -2633,6 +2633,11 @@
return X86Assembler::maxJumpReplacementSize();
}
+ static ptrdiff_t patchableJumpSize()
+ {
+ return X86Assembler::patchableJumpSize();
+ }
+
static bool supportsFloatingPointRounding()
{
if (s_sse4_1CheckState == CPUIDCheckState::NotChecked)
Modified: trunk/Source/_javascript_Core/assembler/X86Assembler.h (205282 => 205283)
--- trunk/Source/_javascript_Core/assembler/X86Assembler.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/assembler/X86Assembler.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -2813,6 +2813,11 @@
{
return 5;
}
+
+ static constexpr ptrdiff_t patchableJumpSize()
+ {
+ return 5;
+ }
#if CPU(X86_64)
static void revertJumpTo_movq_i64r(void* instructionStart, int64_t imm, RegisterID dst)
Modified: trunk/Source/_javascript_Core/jit/JITMathIC.h (205282 => 205283)
--- trunk/Source/_javascript_Core/jit/JITMathIC.h 2016-09-01 07:37:19 UTC (rev 205282)
+++ trunk/Source/_javascript_Core/jit/JITMathIC.h 2016-09-01 08:22:21 UTC (rev 205283)
@@ -81,12 +81,12 @@
// code, it's a win. Second, if the operation does execute, we can emit better code
// once we have an idea about the types of lhs and rhs.
state.slowPathJumps.append(jit.patchableJump());
+ size_t inlineSize = jit.m_assembler.buffer().codeSize() - startSize;
+ ASSERT_UNUSED(inlineSize, static_cast<ptrdiff_t>(inlineSize) <= MacroAssembler::patchableJumpSize());
state.shouldSlowPathRepatch = true;
state.fastPathEnd = jit.label();
ASSERT(!m_generateFastPathOnRepatch); // We should have gathered some observed type info for lhs and rhs before trying to regenerate again.
m_generateFastPathOnRepatch = true;
- size_t inlineSize = jit.m_assembler.buffer().codeSize() - startSize;
- ASSERT_UNUSED(inlineSize, static_cast<ptrdiff_t>(inlineSize) <= MacroAssembler::maxJumpReplacementSize());
return true;
}
}
@@ -96,8 +96,8 @@
switch (result) {
case JITMathICInlineResult::GeneratedFastPath: {
size_t inlineSize = jit.m_assembler.buffer().codeSize() - startSize;
- if (static_cast<ptrdiff_t>(inlineSize) < MacroAssembler::maxJumpReplacementSize()) {
- size_t nopsToEmitInBytes = MacroAssembler::maxJumpReplacementSize() - inlineSize;
+ if (static_cast<ptrdiff_t>(inlineSize) < MacroAssembler::patchableJumpSize()) {
+ size_t nopsToEmitInBytes = MacroAssembler::patchableJumpSize() - inlineSize;
jit.emitNops(nopsToEmitInBytes);
}
state.shouldSlowPathRepatch = true;
