Changeset 96563 in webkit

Timestamp:

Oct 3, 2011 6:16:46 PM (13 years ago)

Author:

barraclough@apple.com

Message:

On X86, switch bucketCount into a register, timeoutCheck into memory
​https://bugs.webkit.org/show_bug.cgi?id=69299

Reviewed by Geoff Garen.

We don't have sufficient registers to keep both in registers, and DFG JIT will trample esi;
it doesn't matter if the bucketCount gets stomped on (in fact it may add to randomness!),
but it if the timeoutCheck gets trashed we may make calls out to the timout_check stub
function too frequently (regressing performance). This patch has no perf impact on sunspider.

• JavaScriptCore.xcodeproj/project.pbxproj:
• assembler/MacroAssemblerX86.h:

(JSC::MacroAssemblerX86::branchAdd32):
(JSC::MacroAssemblerX86::branchSub32):

• Added branchSub32 with AbsoluteAddress.

• jit/JIT.cpp:

(JSC::JIT::emitTimeoutCheck):

• Keep timeout count in memory on X86.

• jit/JITInlineMethods.h:

(JSC::JIT::emitValueProfilingSite):

• remove X86 specific code, switch bucket count back into a register.

• jit/JITStubs.cpp:
  • Stop initializing esi (it is no longer the timeoutCheck!)
• jit/JSInterfaceJIT.h:
  • change definition of esi to be the bucketCountRegister.
• runtime/JSGlobalData.cpp:

(JSC::JSGlobalData::JSGlobalData):

• runtime/JSGlobalData.h:
  • Add timeoutCount as a property to global data (the counter should be per-thread).

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• JavaScriptCore.xcodeproj/project.pbxproj (modified) (2 diffs)
• assembler/MacroAssemblerX86.h (modified) (2 diffs)
• jit/JIT.cpp (modified) (2 diffs)
• jit/JITInlineMethods.h (modified) (3 diffs)
• jit/JITStubs.cpp (modified) (2 diffs)
• jit/JSInterfaceJIT.h (modified) (1 diff)
• runtime/JSGlobalData.cpp (modified) (1 diff)
• runtime/JSGlobalData.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-10-03  Gavin Barraclough  <barraclough@apple.com>
+
+        On X86, switch bucketCount into a register, timeoutCheck into memory
+        https://bugs.webkit.org/show_bug.cgi?id=69299
+
+        Reviewed by Geoff Garen.
+
+        We don't have sufficient registers to keep both in registers, and DFG JIT will trample esi;
+        it doesn't matter if the bucketCount gets stomped on (in fact it may add to randomness!),
+        but it if the timeoutCheck gets trashed we may make calls out to the timout_check stub
+        function too frequently (regressing performance). This patch has no perf impact on sunspider.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * assembler/MacroAssemblerX86.h:
+        (JSC::MacroAssemblerX86::branchAdd32):
+        (JSC::MacroAssemblerX86::branchSub32):
+            - Added branchSub32 with AbsoluteAddress.
+        * jit/JIT.cpp:
+        (JSC::JIT::emitTimeoutCheck):
+            - Keep timeout count in memory on X86.
+        * jit/JITInlineMethods.h:
+        (JSC::JIT::emitValueProfilingSite):
+            - remove X86 specific code, switch bucket count back into a register.
+        * jit/JITStubs.cpp:
+            - Stop initializing esi (it is no longer the timeoutCheck!)
+        * jit/JSInterfaceJIT.h:
+            - change definition of esi to be the bucketCountRegister.
+        * runtime/JSGlobalData.cpp:
+        (JSC::JSGlobalData::JSGlobalData):
+        * runtime/JSGlobalData.h:
+            - Add timeoutCount as a property to global data (the counter should be per-thread).
+
 2011-10-03  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

@@ -1053 +1053 @@
                 860161E10F3A83C100F84710 /* MacroAssemblerX86_64.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MacroAssemblerX86_64.h; sourceTree = "<group>"; };
                 860161E20F3A83C100F84710 /* MacroAssemblerX86Common.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MacroAssemblerX86Common.h; sourceTree = "<group>"; };
+                8604F4F2143A6C4400B295F5 /* ChangeLog */ = {isa = PBXFileReference; lastKnownFileType = text; path = ChangeLog; sourceTree = "<group>"; };
                 8626BECE11928E3900782FAB /* StringStatics.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = StringStatics.cpp; path = text/StringStatics.cpp; sourceTree = "<group>"; };
                 8627E5EA11F1281900A313B5 /* PageAllocation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PageAllocation.h; sourceTree = "<group>"; };
@@ -1550 +1551 @@
                         isa = PBXGroup;
                         children = (
+                                8604F4F2143A6C4400B295F5 /* ChangeLog */,
                                 651122E5140469BA002B101D /* testRegExp.cpp */,
                                 A718F8211178EB4B002465A7 /* create_regex_tables */,

trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86.h

@@ -45 +45 @@
     using MacroAssemblerX86Common::and32;
     using MacroAssemblerX86Common::branchAdd32;
+    using MacroAssemblerX86Common::branchSub32;
     using MacroAssemblerX86Common::sub32;
     using MacroAssemblerX86Common::or32;
@@ -123 +124 @@
     }
 
-    Jump branchAdd32(ResultCondition cond, TrustedImm32 src, AbsoluteAddress dest)
-    {
-        m_assembler.addl_im(src.m_value, dest.m_ptr);
+    Jump branchAdd32(ResultCondition cond, TrustedImm32 imm, AbsoluteAddress dest)
+    {
+        m_assembler.addl_im(imm.m_value, dest.m_ptr);
+        return Jump(m_assembler.jCC(x86Condition(cond)));
+    }
+
+    Jump branchSub32(ResultCondition cond, TrustedImm32 imm, AbsoluteAddress dest)
+    {
+        m_assembler.subl_im(imm.m_value, dest.m_ptr);
         return Jump(m_assembler.jCC(x86Condition(cond)));
     }

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -109 +109 @@
 #endif
 
-#if USE(JSVALUE32_64)
+#if CPU(X86)
+void JIT::emitTimeoutCheck()
+{
+    Jump skipTimeout = branchSub32(NonZero, TrustedImm32(1), AbsoluteAddress(&m_globalData->m_timeoutCount));
+    JITStubCall stubCall(this, cti_timeout_check);
+    stubCall.addArgument(regT1, regT0); // save last result registers.
+    stubCall.call(regT0);
+    store32(regT0, &m_globalData->m_timeoutCount);
+    stubCall.getArgument(0, regT1, regT0); // reload last result registers.
+    skipTimeout.link(this);
+}
+#elif USE(JSVALUE32_64)
 void JIT::emitTimeoutCheck()
 {
@@ -723 +734 @@
 }
 
-#if CPU(X86) && ENABLE(VALUE_PROFILER)
-int bucketCounter = 0;
-#endif
-
 } // namespace JSC
 

trunk/Source/JavaScriptCore/jit/JITInlineMethods.h

@@ -447 +447 @@
 }
 
-#if CPU(X86) && ENABLE(VALUE_PROFILER)
-extern int bucketCounter;
-#endif
-
 #if ENABLE(VALUE_PROFILER)
 inline void JIT::emitValueProfilingSite(ValueProfilingSiteKind siteKind)
@@ -470 +466 @@
     ASSERT(valueProfile);
     
-#if CPU(X86)
-    if (m_randomGenerator.getUint32() & 1)
-        add32(Imm32(1), AbsoluteAddress(&bucketCounter));
-    else
-        add32(Imm32(3), AbsoluteAddress(&bucketCounter));
-    and32(Imm32(ValueProfile::bucketIndexMask), AbsoluteAddress(&bucketCounter));
-    load32(&bucketCounter, scratch);
-    lshift32(TrustedImm32(3), scratch);
-    addPtr(ImmPtr(valueProfile->m_buckets), scratch);
-    storePtr(value, scratch);
-#else
     if (m_randomGenerator.getUint32() & 1)
         add32(Imm32(1), bucketCounterRegister);
@@ -488 +473 @@
     move(ImmPtr(valueProfile->m_buckets), scratch);
     storePtr(value, BaseIndex(scratch, bucketCounterRegister, TimesEight));
-#endif
 }
 #endif

trunk/Source/JavaScriptCore/jit/JITStubs.cpp

@@ -132 +132 @@
     "pushl %ebx" "\n"
     "subl $0x3c, %esp" "\n"
-    "movl $512, %esi" "\n"
     "movl 0x58(%esp), %edi" "\n"
     "call *0x50(%esp)" "\n"
@@ -262 +261 @@
             push ebx;
             sub esp, 0x3c;
-            mov esi, 512;
             mov ecx, esp;
             mov edi, [esp + 0x58];

trunk/Source/JavaScriptCore/jit/JSInterfaceJIT.h

@@ -81 +81 @@
         static const RegisterID firstArgumentRegister = X86Registers::ecx;
         
-        static const RegisterID timeoutCheckRegister = X86Registers::esi;
+        static const RegisterID bucketCounterRegister = X86Registers::esi;
         static const RegisterID callFrameRegister = X86Registers::edi;
         

trunk/Source/JavaScriptCore/runtime/JSGlobalData.cpp

@@ -200 +200 @@
     , exclusiveThread(0)
 #endif
+#if CPU(X86) && ENABLE(JIT)
+    , m_timeoutCount(512)
+#endif
 #if ENABLE(GC_VALIDATION)
     , m_isInitializingObject(false)

trunk/Source/JavaScriptCore/runtime/JSGlobalData.h

@@ -306 +306 @@
 #endif
 
+#if CPU(X86) && ENABLE(JIT)
+        unsigned m_timeoutCount;
+#endif
+
     private:
         JSGlobalData(GlobalDataType, ThreadStackType, HeapSize);