On X86, switch bucketCount into a register, timeoutCheck into memory
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Oct 2011 01:16:46 +0000 (01:16 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Oct 2011 01:16:46 +0000 (01:16 +0000)
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).

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/assembler/MacroAssemblerX86.h
Source/JavaScriptCore/jit/JIT.cpp
Source/JavaScriptCore/jit/JITInlineMethods.h
Source/JavaScriptCore/jit/JITStubs.cpp
Source/JavaScriptCore/jit/JSInterfaceJIT.h
Source/JavaScriptCore/runtime/JSGlobalData.cpp
Source/JavaScriptCore/runtime/JSGlobalData.h

index 5855d09..c1680b2 100644 (file)
@@ -1,3 +1,35 @@
+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>
 
         DFG backends don't have access to per-node predictions from the propagator
index e2f9b49..718fad2 100644 (file)
                860161E00F3A83C100F84710 /* MacroAssemblerX86.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MacroAssemblerX86.h; sourceTree = "<group>"; };
                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>"; };
                863B23DF0FC60E6200703AA4 /* MacroAssemblerCodeRef.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MacroAssemblerCodeRef.h; sourceTree = "<group>"; };
                0867D691FE84028FC02AAC07 /* JavaScriptCore */ = {
                        isa = PBXGroup;
                        children = (
+                               8604F4F2143A6C4400B295F5 /* ChangeLog */,
                                651122E5140469BA002B101D /* testRegExp.cpp */,
                                A718F8211178EB4B002465A7 /* create_regex_tables */,
                                937B63CC09E766D200A671DD /* DerivedSources.make */,
index 64a1437..db701f1 100644 (file)
@@ -44,6 +44,7 @@ public:
     using MacroAssemblerX86Common::add32;
     using MacroAssemblerX86Common::and32;
     using MacroAssemblerX86Common::branchAdd32;
+    using MacroAssemblerX86Common::branchSub32;
     using MacroAssemblerX86Common::sub32;
     using MacroAssemblerX86Common::or32;
     using MacroAssemblerX86Common::load32;
@@ -122,9 +123,15 @@ public:
         m_assembler.movl_rm(src, address);
     }
 
-    Jump branchAdd32(ResultCondition cond, TrustedImm32 src, AbsoluteAddress dest)
+    Jump branchAdd32(ResultCondition cond, TrustedImm32 imm, AbsoluteAddress dest)
     {
-        m_assembler.addl_im(src.m_value, dest.m_ptr);
+        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)));
     }
 
index a6ff0cc..bc5b145 100644 (file)
@@ -108,7 +108,18 @@ void JIT::emitOptimizationCheck(OptimizationCheckKind kind)
 }
 #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()
 {
     Jump skipTimeout = branchSub32(NonZero, TrustedImm32(1), timeoutCheckRegister);
@@ -722,10 +733,6 @@ void JIT::linkFor(JSFunction* callee, CodeBlock* callerCodeBlock, CodeBlock* cal
     repatchBuffer.relink(CodeLocationNearCall(callLinkInfo->callReturnLocation), globalData->jitStubs->ctiVirtualConstruct());
 }
 
-#if CPU(X86) && ENABLE(VALUE_PROFILER)
-int bucketCounter = 0;
-#endif
-
 } // namespace JSC
 
 #endif // ENABLE(JIT)
index 52a0bf4..bee36c4 100644 (file)
@@ -446,10 +446,6 @@ inline void JIT::emitAllocateJSFunction(FunctionExecutable* executable, Register
 #endif
 }
 
-#if CPU(X86) && ENABLE(VALUE_PROFILER)
-extern int bucketCounter;
-#endif
-
 #if ENABLE(VALUE_PROFILER)
 inline void JIT::emitValueProfilingSite(ValueProfilingSiteKind siteKind)
 {
@@ -469,17 +465,6 @@ inline void JIT::emitValueProfilingSite(ValueProfilingSiteKind siteKind)
     
     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);
     else
@@ -487,7 +472,6 @@ inline void JIT::emitValueProfilingSite(ValueProfilingSiteKind siteKind)
     and32(Imm32(ValueProfile::bucketIndexMask), bucketCounterRegister);
     move(ImmPtr(valueProfile->m_buckets), scratch);
     storePtr(value, BaseIndex(scratch, bucketCounterRegister, TimesEight));
-#endif
 }
 #endif
 
index d7d305c..2a6e26b 100644 (file)
@@ -131,7 +131,6 @@ SYMBOL_STRING(ctiTrampoline) ":" "\n"
     "pushl %edi" "\n"
     "pushl %ebx" "\n"
     "subl $0x3c, %esp" "\n"
-    "movl $512, %esi" "\n"
     "movl 0x58(%esp), %edi" "\n"
     "call *0x50(%esp)" "\n"
     "addl $0x3c, %esp" "\n"
@@ -261,7 +260,6 @@ extern "C" {
             push edi;
             push ebx;
             sub esp, 0x3c;
-            mov esi, 512;
             mov ecx, esp;
             mov edi, [esp + 0x58];
             call [esp + 0x50];
index 507b8cb..a5c94ed 100644 (file)
@@ -80,7 +80,7 @@ namespace JSC {
         // OS X if might make more sense to just use regparm.
         static const RegisterID firstArgumentRegister = X86Registers::ecx;
         
-        static const RegisterID timeoutCheckRegister = X86Registers::esi;
+        static const RegisterID bucketCounterRegister = X86Registers::esi;
         static const RegisterID callFrameRegister = X86Registers::edi;
         
         static const RegisterID regT0 = X86Registers::eax;
index b9edb20..9728016 100644 (file)
@@ -199,6 +199,9 @@ JSGlobalData::JSGlobalData(GlobalDataType globalDataType, ThreadStackType thread
 #ifndef NDEBUG
     , exclusiveThread(0)
 #endif
+#if CPU(X86) && ENABLE(JIT)
+    , m_timeoutCount(512)
+#endif
 #if ENABLE(GC_VALIDATION)
     , m_isInitializingObject(false)
 #endif
index d575a1e..b67decd 100644 (file)
@@ -305,6 +305,10 @@ namespace JSC {
         void setInitializingObject(bool);
 #endif
 
+#if CPU(X86) && ENABLE(JIT)
+        unsigned m_timeoutCount;
+#endif
+
     private:
         JSGlobalData(GlobalDataType, ThreadStackType, HeapSize);
         static JSGlobalData*& sharedInstanceInternal();