Fences on x86 should be a lot cheaper
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Sep 2016 21:11:42 +0000 (21:11 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Sep 2016 21:11:42 +0000 (21:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162417

Reviewed by Mark Lam and Geoffrey Garen.
Source/JavaScriptCore:

It turns out that:

    lock; orl $0, (%rsp)

does everything that we wanted from:

    mfence

And it's a lot faster. When I tried mfence for making object visiting concurrent-GC-TSO-
friendly, it was a 9% regression on Octane/splay. But when I tried ortop, it was neutral.
So, we should use ortop from now on.

This part of the change is for the JITs. MacroAssembler::memoryFence() appears to always
mean something like an acqrel fence, so it's safe to make this use ortop. Since B3's Fence
compiles to Air MemoryFence, which is just MacroAssembler::memoryFence(), this also changes
B3 codegen.

* assembler/MacroAssemblerX86Common.h:
(JSC::MacroAssemblerX86Common::memoryFence):
* assembler/X86Assembler.h:
(JSC::X86Assembler::lock):
* b3/testb3.cpp:
(JSC::B3::testX86MFence):
(JSC::B3::testX86CompilerFence):

Source/WTF:

It turns out that:

    lock; orl $0, (%rsp)

does everything that we wanted from:

    mfence

And it's a lot faster. When I tried mfence for making object visiting concurrent-GC-TSO-
friendly, it was a 9% regression on Octane/splay. But when I tried ortop, it was neutral.
So, we should use ortop from now on.

This part of the change just affects our Atomics. I also changed this in the JITs.

* wtf/Atomics.h:
(WTF::x86_ortop):
(WTF::storeLoadFence):
(WTF::x86_mfence): Deleted.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
Source/JavaScriptCore/assembler/X86Assembler.h
Source/JavaScriptCore/b3/testb3.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/Atomics.h

index 02c444c..3b355b0 100644 (file)
@@ -1,3 +1,35 @@
+2016-09-22  Filip Pizlo  <fpizlo@apple.com>
+
+        Fences on x86 should be a lot cheaper
+        https://bugs.webkit.org/show_bug.cgi?id=162417
+
+        Reviewed by Mark Lam and Geoffrey Garen.
+
+        It turns out that:
+        
+            lock; orl $0, (%rsp)
+        
+        does everything that we wanted from:
+        
+            mfence
+        
+        And it's a lot faster. When I tried mfence for making object visiting concurrent-GC-TSO-
+        friendly, it was a 9% regression on Octane/splay. But when I tried ortop, it was neutral.
+        So, we should use ortop from now on.
+        
+        This part of the change is for the JITs. MacroAssembler::memoryFence() appears to always
+        mean something like an acqrel fence, so it's safe to make this use ortop. Since B3's Fence
+        compiles to Air MemoryFence, which is just MacroAssembler::memoryFence(), this also changes
+        B3 codegen.
+
+        * assembler/MacroAssemblerX86Common.h:
+        (JSC::MacroAssemblerX86Common::memoryFence):
+        * assembler/X86Assembler.h:
+        (JSC::X86Assembler::lock):
+        * b3/testb3.cpp:
+        (JSC::B3::testX86MFence):
+        (JSC::B3::testX86CompilerFence):
+
 2016-09-22  Joseph Pecoraro  <pecoraro@apple.com>
 
         test262: Function length should be number of parameters before parameters with default values
index 6d5c086..6fcd74d 100644 (file)
@@ -2629,9 +2629,12 @@ public:
         m_assembler.nop();
     }
     
+    // We take memoryFence to mean acqrel. This has acqrel semantics on x86.
     void memoryFence()
     {
-        m_assembler.mfence();
+        // lock; orl $0, (%rsp)
+        m_assembler.lock();
+        m_assembler.orl_im(0, 0, X86Registers::esp);
     }
 
     static void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
index b83c0ee..9e60990 100644 (file)
@@ -249,6 +249,7 @@ private:
         OP_ESCAPE_DD                    = 0xDD,
         OP_CALL_rel32                   = 0xE8,
         OP_JMP_rel32                    = 0xE9,
+        PRE_LOCK                        = 0xF0,
         PRE_SSE_F2                      = 0xF2,
         PRE_SSE_F3                      = 0xF3,
         OP_HLT                          = 0xF4,
@@ -2684,6 +2685,11 @@ public:
         m_formatter.prefix(PRE_PREDICT_BRANCH_NOT_TAKEN);
     }
     
+    void lock()
+    {
+        m_formatter.prefix(PRE_LOCK);
+    }
+    
     void mfence()
     {
         m_formatter.threeByteOp(OP2_3BYTE_ESCAPE_AE, OP3_MFENCE);
index 203b5d5..7f74ed7 100644 (file)
@@ -13062,7 +13062,8 @@ void testX86MFence()
     root->appendNew<Value>(proc, Return, Origin());
     
     auto code = compile(proc);
-    checkUsesInstruction(*code, "mfence");
+    checkUsesInstruction(*code, "lock or $0x0, (%rsp)");
+    checkDoesNotUseInstruction(*code, "mfence");
 }
 
 void testX86CompilerFence()
@@ -13075,6 +13076,7 @@ void testX86CompilerFence()
     root->appendNew<Value>(proc, Return, Origin());
     
     auto code = compile(proc);
+    checkDoesNotUseInstruction(*code, "lock");
     checkDoesNotUseInstruction(*code, "mfence");
 }
 
index cf36426..3011bc5 100644 (file)
@@ -1,3 +1,29 @@
+2016-09-22  Filip Pizlo  <fpizlo@apple.com>
+
+        Fences on x86 should be a lot cheaper
+        https://bugs.webkit.org/show_bug.cgi?id=162417
+
+        Reviewed by Mark Lam and Geoffrey Garen.
+        
+        It turns out that:
+        
+            lock; orl $0, (%rsp)
+        
+        does everything that we wanted from:
+        
+            mfence
+        
+        And it's a lot faster. When I tried mfence for making object visiting concurrent-GC-TSO-
+        friendly, it was a 9% regression on Octane/splay. But when I tried ortop, it was neutral.
+        So, we should use ortop from now on.
+        
+        This part of the change just affects our Atomics. I also changed this in the JITs.
+
+        * wtf/Atomics.h:
+        (WTF::x86_ortop):
+        (WTF::storeLoadFence):
+        (WTF::x86_mfence): Deleted.
+
 2016-09-21  Alexey Proskuryakov  <ap@apple.com>
 
         Rolling out r206244, as it caused flaky crashes on tests.
index 9b47a0e..c48879f 100644 (file)
@@ -167,7 +167,7 @@ inline void memoryBarrierBeforeUnlock() { arm_dmb(); }
 
 #elif CPU(X86) || CPU(X86_64)
 
-inline void x86_mfence()
+inline void x86_ortop()
 {
 #if OS(WINDOWS)
     // I think that this does the equivalent of a dummy interlocked instruction,
@@ -176,13 +176,15 @@ inline void x86_mfence()
     // investigate if that is actually better.
     MemoryBarrier();
 #else
-    asm volatile("mfence" ::: "memory");
+    // This has acqrel semantics and is much cheaper than mfence. For exampe, in the JSC GC, using
+    // mfence as a store-load fence was a 9% slow-down on Octane/splay while using this was neutral.
+    asm volatile("lock; orl $0, (%%rsp)" ::: "memory");
 #endif
 }
 
 inline void loadLoadFence() { compilerFence(); }
 inline void loadStoreFence() { compilerFence(); }
-inline void storeLoadFence() { x86_mfence(); }
+inline void storeLoadFence() { x86_ortop(); }
 inline void storeStoreFence() { compilerFence(); }
 inline void memoryBarrierAfterLock() { compilerFence(); }
 inline void memoryBarrierBeforeUnlock() { compilerFence(); }