JITMathIC was misusing maxJumpReplacementSize
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Sep 2016 08:22:21 +0000 (08:22 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Sep 2016 08:22:21 +0000 (08:22 +0000)
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):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/ARM64Assembler.h
Source/JavaScriptCore/assembler/ARMv7Assembler.h
Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
Source/JavaScriptCore/assembler/X86Assembler.h
Source/JavaScriptCore/jit/JITMathIC.h

index 52681a8..d076f35 100644 (file)
@@ -1,3 +1,36 @@
+2016-09-01  Saam Barati  <sbarati@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.tea@gmail.com>
 
         [JSC] Add initiator parameter to module pipeline
index 86f13ed..28b9fd7 100644 (file)
@@ -2512,6 +2512,11 @@ public:
     {
         return 4;
     }
+
+    static constexpr ptrdiff_t patchableJumpSize()
+    {
+        return 4;
+    }
     
     static void replaceWithLoad(void* where)
     {
index af076e8..94f25c0 100644 (file)
@@ -2340,6 +2340,11 @@ public:
         return 4;
 #endif
     }
+
+    static constexpr ptrdiff_t patchableJumpSize()
+    {
+        return 10;
+    }
     
     static void replaceWithLoad(void* instructionStart)
     {
index 277c052..d306f62 100644 (file)
@@ -3249,6 +3249,11 @@ public:
         return ARM64Assembler::maxJumpReplacementSize();
     }
 
+    static ptrdiff_t patchableJumpSize()
+    {
+        return ARM64Assembler::patchableJumpSize();
+    }
+
     RegisterID scratchRegisterForBlinding()
     {
         // We *do not* have a scratch register for blinding.
index 4513ed2..49bc2d1 100644 (file)
@@ -1350,6 +1350,11 @@ public:
         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
index 0492b14..0b067ec 100644 (file)
@@ -2633,6 +2633,11 @@ public:
         return X86Assembler::maxJumpReplacementSize();
     }
 
+    static ptrdiff_t patchableJumpSize()
+    {
+        return X86Assembler::patchableJumpSize();
+    }
+
     static bool supportsFloatingPointRounding()
     {
         if (s_sse4_1CheckState == CPUIDCheckState::NotChecked)
index 4cbb3d5..b83c0ee 100644 (file)
@@ -2813,6 +2813,11 @@ public:
     {
         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)
index afd7d13..2f2de9c 100644 (file)
@@ -81,12 +81,12 @@ public:
                 // 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 @@ public:
         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;