Changeset 205283 in webkit

Timestamp:

Sep 1, 2016 1:22:21 AM (8 years ago)

Author:

sbarati@apple.com

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/ARM64Assembler.h (modified) (1 diff)
• assembler/ARMv7Assembler.h (modified) (1 diff)
• assembler/MacroAssemblerARM64.h (modified) (1 diff)
• assembler/MacroAssemblerARMv7.h (modified) (1 diff)
• assembler/MacroAssemblerX86Common.h (modified) (1 diff)
• assembler/X86Assembler.h (modified) (1 diff)
• jit/JITMathIC.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/assembler/ARM64Assembler.h

@@ -2513 +2513 @@
         return 4;
     }
+
+    static constexpr ptrdiff_t patchableJumpSize()
+    {
+        return 4;
+    }
     
     static void replaceWithLoad(void* where)

trunk/Source/JavaScriptCore/assembler/ARMv7Assembler.h

@@ -2341 +2341 @@
 #endif
     }
+
+    static constexpr ptrdiff_t patchableJumpSize()
+    {
+        return 10;
+    }
     
     static void replaceWithLoad(void* instructionStart)

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -3250 +3250 @@
     }
 
+    static ptrdiff_t patchableJumpSize()
+    {
+        return ARM64Assembler::patchableJumpSize();
+    }
+
     RegisterID scratchRegisterForBlinding()
     {

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

@@ -1351 +1351 @@
     }
 
+    static ptrdiff_t patchableJumpSize()
+    {
+        return ARMv7Assembler::patchableJumpSize();
+    }
+
     // Forwards / external control flow operations:
     //

trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h

@@ -2632 +2632 @@
     {
         return X86Assembler::maxJumpReplacementSize();
+    }
+
+    static ptrdiff_t patchableJumpSize()
+    {
+        return X86Assembler::patchableJumpSize();
     }
 

trunk/Source/JavaScriptCore/assembler/X86Assembler.h

@@ -2814 +2814 @@
         return 5;
     }
+
+    static constexpr ptrdiff_t patchableJumpSize()
+    {
+        return 5;
+    }
     
 #if CPU(X86_64)

trunk/Source/JavaScriptCore/jit/JITMathIC.h

@@ -82 +82 @@
                 // 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;
             }
@@ -97 +97 @@
         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);
             }