Changeset 255987 in webkit

Timestamp:

Feb 6, 2020, 3:25:01 PM (6 years ago)

Author:

[email protected]

Message:

[JSC] CodeBlock::shrinkToFit should shrink m_constantRegisters and m_constantsSourceCodeRepresentation in 64bit architectures
​https://bugs.webkit.org/show_bug.cgi?id=207356

Reviewed by Mark Lam.

Only 32bit architectures are using m_constantRegisters's address. 64bit architectures are not relying on m_constantRegisters's address.
This patches fixes the thing so that CodeBlock::shrinkToFit will shrink m_constantRegisters and m_constantsSourceCodeRepresentation
regardless of whether this is EarlyShrink or not. We also move DFG/FTL's LateShrink call to the place after calling DFGCommon reallyAdd
since they can add more constant registers.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::shrinkToFit):

• bytecode/CodeBlock.h:
• dfg/DFGJITCompiler.cpp:

(JSC::DFG::JITCompiler::compile):
(JSC::DFG::JITCompiler::compileFunction):

• dfg/DFGJITFinalizer.cpp:

(JSC::DFG::JITFinalizer::finalizeCommon):

• dfg/DFGPlan.cpp:

(JSC::DFG::Plan::compileInThreadImpl):
(JSC::DFG::Plan::finalizeWithoutNotifyingCallback):

• jit/JIT.cpp:

(JSC::JIT::link):

• jit/JIT.h:
• jit/JITInlines.h:

(JSC::JIT::emitLoadDouble):
(JSC::JIT::emitLoadInt32ToDouble): Deleted.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/CodeBlock.cpp (modified) (1 diff)
• bytecode/CodeBlock.h (modified) (2 diffs)
• dfg/DFGJITCompiler.cpp (modified) (2 diffs)
• dfg/DFGJITFinalizer.cpp (modified) (1 diff)
• dfg/DFGPlan.cpp (modified) (2 diffs)
• jit/JIT.cpp (modified) (1 diff)
• jit/JIT.h (modified) (2 diffs)
• jit/JITInlines.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-02-06  Yusuke Suzuki  <[email protected]>
+
+        [JSC] CodeBlock::shrinkToFit should shrink m_constantRegisters and m_constantsSourceCodeRepresentation in 64bit architectures
+        https://bugs.webkit.org/show_bug.cgi?id=207356
+
+        Reviewed by Mark Lam.
+
+        Only 32bit architectures are using m_constantRegisters's address. 64bit architectures are not relying on m_constantRegisters's address.
+        This patches fixes the thing so that CodeBlock::shrinkToFit will shrink m_constantRegisters and m_constantsSourceCodeRepresentation
+        regardless of whether this is EarlyShrink or not. We also move DFG/FTL's LateShrink call to the place after calling DFGCommon reallyAdd
+        since they can add more constant registers.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::shrinkToFit):
+        * bytecode/CodeBlock.h:
+        * dfg/DFGJITCompiler.cpp:
+        (JSC::DFG::JITCompiler::compile):
+        (JSC::DFG::JITCompiler::compileFunction):
+        * dfg/DFGJITFinalizer.cpp:
+        (JSC::DFG::JITFinalizer::finalizeCommon):
+        * dfg/DFGPlan.cpp:
+        (JSC::DFG::Plan::compileInThreadImpl):
+        (JSC::DFG::Plan::finalizeWithoutNotifyingCallback):
+        * jit/JIT.cpp:
+        (JSC::JIT::link):
+        * jit/JIT.h:
+        * jit/JITInlines.h:
+        (JSC::JIT::emitLoadDouble):
+        (JSC::JIT::emitLoadInt32ToDouble): Deleted.
+
 2020-02-05  Don Olmstead  <[email protected]>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1954 +1954 @@
 }
 
-void CodeBlock::shrinkToFit(ShrinkMode shrinkMode)
+void CodeBlock::shrinkToFit(const ConcurrentJSLocker&, ShrinkMode shrinkMode)
 {
     ConcurrentJSLocker locker(m_lock);
 
-    if (shrinkMode == EarlyShrink) {
+#if USE(JSVALUE32_64)
+    // Only 32bit Baseline JIT is touching m_constantRegisters address directly.
+    if (shrinkMode == ShrinkMode::EarlyShrink)
         m_constantRegisters.shrinkToFit();
-        m_constantsSourceCodeRepresentation.shrinkToFit();
-        
+#else
+    m_constantRegisters.shrinkToFit();
+#endif
+    m_constantsSourceCodeRepresentation.shrinkToFit();
+
+    if (shrinkMode == ShrinkMode::EarlyShrink) {
         if (m_rareData) {
             m_rareData->m_switchJumpTables.shrinkToFit();

trunk/Source/JavaScriptCore/bytecode/CodeBlock.h

@@ -636 +636 @@
     DirectEvalCodeCache& directEvalCodeCache() { createRareDataIfNecessary(); return m_rareData->m_directEvalCodeCache; }
 
-    enum ShrinkMode {
+    enum class ShrinkMode {
         // Shrink prior to generating machine code that may point directly into vectors.
         EarlyShrink,
@@ -643 +643 @@
         // and appending to others. At this time it is not safe to shrink certain vectors
         // because we would have generated machine code that references them directly.
-        LateShrink
+        LateShrink,
     };
-    void shrinkToFit(ShrinkMode);
+    void shrinkToFit(const ConcurrentJSLocker&, ShrinkMode);
 
     // Functions for controlling when JITting kicks in, in a mixed mode

trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp

@@ -393 +393 @@
     m_speculative->linkOSREntries(*linkBuffer);
 
-    codeBlock()->shrinkToFit(CodeBlock::LateShrink);
-
     disassemble(*linkBuffer);
 
@@ -494 +492 @@
     m_speculative->linkOSREntries(*linkBuffer);
     
-    codeBlock()->shrinkToFit(CodeBlock::LateShrink);
-
     if (requiresArityFixup)
         linkBuffer->link(callArityFixup, FunctionPtr<JITThunkPtrTag>(vm().getCTIStub(arityFixupGenerator).code()));

trunk/Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp

@@ -84 +84 @@
     CodeBlock* codeBlock = m_plan.codeBlock();
 
-    // Some JIT finalizers may have added more constants. Shrink-to-fit those things now.
-    {
-        ConcurrentJSLocker locker(codeBlock->m_lock);
-        codeBlock->constants().shrinkToFit();
-        codeBlock->constantsSourceCodeRepresentation().shrinkToFit();
-    }
-
 #if ENABLE(FTL_JIT)
     m_jitCode->optimizeAfterWarmUp(codeBlock);

trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp

@@ -280 +280 @@
     // powerful than a late one. It's safe to do so because we haven't generated any code
     // that references any of the tables directly, yet.
-    m_codeBlock->shrinkToFit(CodeBlock::EarlyShrink);
+    {
+        ConcurrentJSLocker locker(m_codeBlock->m_lock);
+        m_codeBlock->shrinkToFit(locker, CodeBlock::ShrinkMode::EarlyShrink);
+    }
 
     if (validationEnabled())
@@ -618 +621 @@
             ConcurrentJSLocker locker(m_codeBlock->m_lock);
             m_codeBlock->jitCode()->shrinkToFit(locker);
+            m_codeBlock->shrinkToFit(locker, CodeBlock::ShrinkMode::LateShrink);
         }
 

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -944 +944 @@
         static_cast<double>(m_codeBlock->instructionsSize()));
 
-    m_codeBlock->shrinkToFit(CodeBlock::LateShrink);
+    {
+        ConcurrentJSLocker locker(m_codeBlock->m_lock);
+        m_codeBlock->shrinkToFit(locker, CodeBlock::ShrinkMode::LateShrink);
+    }
     m_codeBlock->setJITCode(
         adoptRef(*new DirectJITCode(result, withArityCheck, JITType::BaselineJIT)));

trunk/Source/JavaScriptCore/jit/JIT.h

@@ -338 +338 @@
         bool isOperandConstantDouble(VirtualRegister);
         
-        void emitLoadDouble(VirtualRegister, FPRegisterID value);
-        void emitLoadInt32ToDouble(VirtualRegister, FPRegisterID value);
-
         enum WriteBarrierMode { UnconditionalWriteBarrier, ShouldFilterBase, ShouldFilterValue, ShouldFilterBaseAndValue };
         // value register in write barrier is used before any scratch registers
@@ -418 +415 @@
         bool getOperandConstantInt(VirtualRegister op1, VirtualRegister op2, VirtualRegister& op, int32_t& constant);
 
+        void emitLoadDouble(VirtualRegister, FPRegisterID value);
         void emitLoadTag(VirtualRegister, RegisterID tag);
         void emitLoadPayload(VirtualRegister, RegisterID payload);

trunk/Source/JavaScriptCore/jit/JITInlines.h

@@ -378 +378 @@
 #if USE(JSVALUE32_64)
 
+inline void JIT::emitLoadDouble(VirtualRegister reg, FPRegisterID value)
+{
+    if (reg.isConstant()) {
+        WriteBarrier<Unknown>& inConstantPool = m_codeBlock->constantRegister(reg);
+        loadDouble(TrustedImmPtr(&inConstantPool), value);
+    } else
+        loadDouble(addressFor(reg), value);
+}
+
 inline void JIT::emitLoadTag(VirtualRegister reg, RegisterID tag)
 {
@@ -439 +448 @@
     emitLoad(reg2, tag2, payload2);
     emitLoad(reg1, tag1, payload1);
-}
-
-inline void JIT::emitLoadDouble(VirtualRegister reg, FPRegisterID value)
-{
-    if (reg.isConstant()) {
-        WriteBarrier<Unknown>& inConstantPool = m_codeBlock->constantRegister(reg);
-        loadDouble(TrustedImmPtr(&inConstantPool), value);
-    } else
-        loadDouble(addressFor(reg), value);
-}
-
-inline void JIT::emitLoadInt32ToDouble(VirtualRegister reg, FPRegisterID value)
-{
-    if (reg.isConstant()) {
-        WriteBarrier<Unknown>& inConstantPool = m_codeBlock->constantRegister(reg);
-        char* bytePointer = reinterpret_cast<char*>(&inConstantPool);
-        convertInt32ToDouble(AbsoluteAddress(bytePointer + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), value);
-    } else
-        convertInt32ToDouble(payloadFor(reg), value);
 }
 
@@ -615 +605 @@
 }
 
-inline void JIT::emitLoadDouble(VirtualRegister reg, FPRegisterID value)
-{
-    if (reg.isConstant()) {
-        WriteBarrier<Unknown>& inConstantPool = m_codeBlock->constantRegister(reg);
-        loadDouble(TrustedImmPtr(&inConstantPool), value);
-    } else
-        loadDouble(addressFor(reg), value);
-}
-
-inline void JIT::emitLoadInt32ToDouble(VirtualRegister reg, FPRegisterID value)
-{
-    if (reg.isConstant()) {
-        ASSERT(isOperandConstantInt(reg));
-        convertInt32ToDouble(Imm32(getConstantOperand(reg).asInt32()), value);
-    } else
-        convertInt32ToDouble(addressFor(reg), value);
-}
-
 ALWAYS_INLINE JIT::PatchableJump JIT::emitPatchableJumpIfNotInt(RegisterID reg)
 {