Attachment 284521 Details for Bug 160171 – the patch

[patch] the patch

blah.patch (text/plain), 17.89 KB, created by Filip Pizlo on 2016-07-25 13:31:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2016-07-25 13:31:44 PDT

Size: 17.89 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 203695)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,41 @@
>+2016-07-25  Filip Pizlo  <fpizlo@apple.com>
>+
>+        AssemblyHelpers should own all of the cell allocation methods
>+        https://bugs.webkit.org/show_bug.cgi?id=160171
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Prior to this change we had some code in DFGSpeculativeJIT.h and some code in JIT.h that
>+        did cell allocation.
>+        
>+        This change moves all of that code into AssemblyHelpers.h.
>+
>+        * dfg/DFGSpeculativeJIT.h:
>+        (JSC::DFG::SpeculativeJIT::emitAllocateJSCell):
>+        (JSC::DFG::SpeculativeJIT::emitAllocateJSObject):
>+        (JSC::DFG::SpeculativeJIT::emitAllocateJSObjectWithKnownSize):
>+        (JSC::DFG::SpeculativeJIT::emitAllocateVariableSizedJSObject):
>+        (JSC::DFG::SpeculativeJIT::emitAllocateDestructibleObject):
>+        * jit/AssemblyHelpers.h:
>+        (JSC::AssemblyHelpers::emitAllocate):
>+        (JSC::AssemblyHelpers::emitAllocateJSCell):
>+        (JSC::AssemblyHelpers::emitAllocateJSObject):
>+        (JSC::AssemblyHelpers::emitAllocateJSObjectWithKnownSize):
>+        (JSC::AssemblyHelpers::emitAllocateVariableSized):
>+        (JSC::AssemblyHelpers::emitAllocateVariableSizedJSObject):
>+        (JSC::AssemblyHelpers::emitAllocateDestructibleObject):
>+        * jit/JIT.h:
>+        * jit/JITInlines.h:
>+        (JSC::JIT::isOperandConstantChar):
>+        (JSC::JIT::emitValueProfilingSite):
>+        (JSC::JIT::emitAllocateJSObject): Deleted.
>+        * jit/JITOpcodes.cpp:
>+        (JSC::JIT::emit_op_new_object):
>+        (JSC::JIT::emit_op_create_this):
>+        * jit/JITOpcodes32_64.cpp:
>+        (JSC::JIT::emit_op_new_object):
>+        (JSC::JIT::emit_op_create_this):
>+
> 2016-07-25  Saam Barati  <sbarati@apple.com>
> 
>         op_mul/ArithMul(Untyped,Untyped) should be an IC
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	(revision 203694)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	(working copy)
>@@ -2546,20 +2546,7 @@ public:
>     void emitAllocateJSCell(GPRReg resultGPR, GPRReg allocatorGPR, StructureType structure,
>         GPRReg scratchGPR, MacroAssembler::JumpList& slowPath)
>     {
>-        if (Options::forceGCSlowPaths())
>-            slowPath.append(m_jit.jump());
>-        else {
>-            m_jit.loadPtr(MacroAssembler::Address(allocatorGPR, MarkedAllocator::offsetOfFreeListHead()), resultGPR);
>-            slowPath.append(m_jit.branchTestPtr(MacroAssembler::Zero, resultGPR));
>-        }
>-        
>-        // The object is half-allocated: we have what we know is a fresh object, but
>-        // it's still on the GC's free list.
>-        m_jit.loadPtr(MacroAssembler::Address(resultGPR), scratchGPR);
>-        m_jit.storePtr(scratchGPR, MacroAssembler::Address(allocatorGPR, MarkedAllocator::offsetOfFreeListHead()));
>-
>-        // Initialize the object's Structure.
>-        m_jit.emitStoreStructureWithTypeInfo(structure, resultGPR, scratchGPR);
>+        m_jit.emitAllocateJSCell(resultGPR, allocatorGPR, structure, scratchGPR, slowPath);
>     }
> 
>     // Allocator for an object of a specific size.
>@@ -2567,10 +2554,7 @@ public:
>     void emitAllocateJSObject(GPRReg resultGPR, GPRReg allocatorGPR, StructureType structure,
>         StorageType storage, GPRReg scratchGPR, MacroAssembler::JumpList& slowPath)
>     {
>-        emitAllocateJSCell(resultGPR, allocatorGPR, structure, scratchGPR, slowPath);
>-        
>-        // Initialize the object's property storage pointer.
>-        m_jit.storePtr(storage, MacroAssembler::Address(resultGPR, JSObject::butterflyOffset()));
>+        m_jit.emitAllocateJSObject(resultGPR, allocatorGPR, structure, storage, scratchGPR, slowPath);
>     }
> 
>     template <typename ClassType, typename StructureType, typename StorageType> // StructureType and StorageType can be GPR or ImmPtr.
>@@ -2578,9 +2562,7 @@ public:
>         GPRReg resultGPR, StructureType structure, StorageType storage, GPRReg scratchGPR1,
>         GPRReg scratchGPR2, MacroAssembler::JumpList& slowPath, size_t size)
>     {
>-        MarkedAllocator* allocator = &m_jit.vm()->heap.allocatorForObjectOfType<ClassType>(size);
>-        m_jit.move(TrustedImmPtr(allocator), scratchGPR1);
>-        emitAllocateJSObject(resultGPR, scratchGPR1, structure, storage, scratchGPR2, slowPath);
>+        m_jit.emitAllocateJSObjectWithKnownSize<ClassType>(resultGPR, structure, storage, scratchGPR1, scratchGPR2, slowPath, size);
>     }
> 
>     // Convenience allocator for a built-in object.
>@@ -2588,42 +2570,20 @@ public:
>     void emitAllocateJSObject(GPRReg resultGPR, StructureType structure, StorageType storage,
>         GPRReg scratchGPR1, GPRReg scratchGPR2, MacroAssembler::JumpList& slowPath)
>     {
>-        emitAllocateJSObjectWithKnownSize<ClassType>(
>-            resultGPR, structure, storage, scratchGPR1, scratchGPR2, slowPath,
>-            ClassType::allocationSize(0));
>+        m_jit.emitAllocateJSObject<ClassType>(resultGPR, structure, storage, scratchGPR1, scratchGPR2, slowPath);
>     }
> 
>     template <typename ClassType, typename StructureType> // StructureType and StorageType can be GPR or ImmPtr.
>     void emitAllocateVariableSizedJSObject(GPRReg resultGPR, StructureType structure, GPRReg allocationSize, GPRReg scratchGPR1, GPRReg scratchGPR2, MacroAssembler::JumpList& slowPath)
>     {
>-        static_assert(!(MarkedSpace::preciseStep & (MarkedSpace::preciseStep - 1)), "MarkedSpace::preciseStep must be a power of two.");
>-        static_assert(!(MarkedSpace::impreciseStep & (MarkedSpace::impreciseStep - 1)), "MarkedSpace::impreciseStep must be a power of two.");
>-
>-        MarkedSpace::Subspace& subspace = m_jit.vm()->heap.subspaceForObjectOfType<ClassType>();
>-        m_jit.add32(TrustedImm32(MarkedSpace::preciseStep - 1), allocationSize);
>-        MacroAssembler::Jump notSmall = m_jit.branch32(MacroAssembler::AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::preciseCutoff));
>-        m_jit.rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::preciseStep)), scratchGPR1);
>-        m_jit.mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
>-        m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace.preciseAllocators[0]), scratchGPR1);
>-
>-        MacroAssembler::Jump selectedSmallSpace = m_jit.jump();
>-        notSmall.link(&m_jit);
>-        slowPath.append(m_jit.branch32(MacroAssembler::AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::impreciseCutoff)));
>-        m_jit.rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::impreciseStep)), scratchGPR1);
>-        m_jit.mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
>-        m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace.impreciseAllocators[0]), scratchGPR1);
>-
>-        selectedSmallSpace.link(&m_jit);
>-
>-        emitAllocateJSObject(resultGPR, scratchGPR1, structure, TrustedImmPtr(0), scratchGPR2, slowPath);
>+        m_jit.emitAllocateVariableSizedJSObject<ClassType>(resultGPR, structure, allocationSize, scratchGPR1, scratchGPR2, slowPath);
>     }
> 
>-    template <typename T>
>+    template<typename ClassType>
>     void emitAllocateDestructibleObject(GPRReg resultGPR, Structure* structure, 
>         GPRReg scratchGPR1, GPRReg scratchGPR2, MacroAssembler::JumpList& slowPath)
>     {
>-        emitAllocateJSObject<T>(resultGPR, TrustedImmPtr(structure), TrustedImmPtr(0), scratchGPR1, scratchGPR2, slowPath);
>-        m_jit.storePtr(TrustedImmPtr(structure->classInfo()), MacroAssembler::Address(resultGPR, JSDestructibleObject::classInfoOffset()));
>+        m_jit.emitAllocateDestructibleObject<ClassType>(resultGPR, structure, scratchGPR1, scratchGPR2, slowPath);
>     }
> 
>     void emitAllocateRawObject(GPRReg resultGPR, Structure*, GPRReg storageGPR, unsigned numElements, unsigned vectorLength);
>Index: Source/JavaScriptCore/jit/AssemblyHelpers.h
>===================================================================
>--- Source/JavaScriptCore/jit/AssemblyHelpers.h	(revision 203694)
>+++ Source/JavaScriptCore/jit/AssemblyHelpers.h	(working copy)
>@@ -32,9 +32,11 @@
> #include "CopyBarrier.h"
> #include "FPRInfo.h"
> #include "GPRInfo.h"
>+#include "Heap.h"
> #include "InlineCallFrame.h"
> #include "JITCode.h"
> #include "MacroAssembler.h"
>+#include "MarkedSpace.h"
> #include "MaxFrameExtentForSlowPathCall.h"
> #include "RegisterAtOffsetList.h"
> #include "RegisterSet.h"
>@@ -1404,6 +1406,90 @@ public:
>     void emitRandomThunk(GPRReg scratch0, GPRReg scratch1, GPRReg scratch2, GPRReg scratch3, FPRReg result);
> #endif
>     
>+    void emitAllocate(GPRReg resultGPR, GPRReg allocatorGPR, GPRReg scratchGPR, JumpList& slowPath)
>+    {
>+        if (Options::forceGCSlowPaths())
>+            slowPath.append(jump());
>+        else {
>+            loadPtr(Address(allocatorGPR, MarkedAllocator::offsetOfFreeListHead()), resultGPR);
>+            slowPath.append(branchTestPtr(Zero, resultGPR));
>+        }
>+        
>+        // The object is half-allocated: we have what we know is a fresh object, but
>+        // it's still on the GC's free list.
>+        loadPtr(Address(resultGPR), scratchGPR);
>+        storePtr(scratchGPR, Address(allocatorGPR, MarkedAllocator::offsetOfFreeListHead()));
>+    }
>+    
>+    template<typename StructureType>
>+    void emitAllocateJSCell(GPRReg resultGPR, GPRReg allocatorGPR, StructureType structure, GPRReg scratchGPR, JumpList& slowPath)
>+    {
>+        emitAllocate(resultGPR, allocatorGPR, scratchGPR, slowPath);
>+        emitStoreStructureWithTypeInfo(structure, resultGPR, scratchGPR);
>+    }
>+    
>+    template<typename StructureType, typename StorageType>
>+    void emitAllocateJSObject(GPRReg resultGPR, GPRReg allocatorGPR, StructureType structure, StorageType storage, GPRReg scratchGPR, JumpList& slowPath)
>+    {
>+        emitAllocateJSCell(resultGPR, allocatorGPR, structure, scratchGPR, slowPath);
>+        storePtr(storage, Address(resultGPR, JSObject::butterflyOffset()));
>+    }
>+    
>+    template<typename ClassType, typename StructureType, typename StorageType>
>+    void emitAllocateJSObjectWithKnownSize(
>+        GPRReg resultGPR, StructureType structure, StorageType storage, GPRReg scratchGPR1,
>+        GPRReg scratchGPR2, JumpList& slowPath, size_t size)
>+    {
>+        MarkedAllocator* allocator = &vm()->heap.allocatorForObjectOfType<ClassType>(size);
>+        move(TrustedImmPtr(allocator), scratchGPR1);
>+        emitAllocateJSObject(resultGPR, scratchGPR1, structure, storage, scratchGPR2, slowPath);
>+    }
>+    
>+    template<typename ClassType, typename StructureType, typename StorageType>
>+    void emitAllocateJSObject(GPRReg resultGPR, StructureType structure, StorageType storage, GPRReg scratchGPR1, GPRReg scratchGPR2, JumpList& slowPath)
>+    {
>+        emitAllocateJSObjectWithKnownSize<ClassType>(resultGPR, structure, storage, scratchGPR1, scratchGPR2, slowPath, ClassType::allocationSize(0));
>+    }
>+    
>+    void emitAllocateVariableSized(GPRReg resultGPR, MarkedSpace::Subspace& subspace, GPRReg allocationSize, GPRReg scratchGPR1, GPRReg scratchGPR2, JumpList& slowPath)
>+    {
>+        static_assert(!(MarkedSpace::preciseStep & (MarkedSpace::preciseStep - 1)), "MarkedSpace::preciseStep must be a power of two.");
>+        static_assert(!(MarkedSpace::impreciseStep & (MarkedSpace::impreciseStep - 1)), "MarkedSpace::impreciseStep must be a power of two.");
>+        
>+        add32(TrustedImm32(MarkedSpace::preciseStep - 1), allocationSize);
>+        Jump notSmall = branch32(AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::preciseCutoff));
>+        rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::preciseStep)), scratchGPR1);
>+        mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
>+        addPtr(TrustedImmPtr(&subspace.preciseAllocators[0]), scratchGPR1);
>+
>+        Jump selectedSmallSpace = jump();
>+        notSmall.link(this);
>+        slowPath.append(branch32(AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::impreciseCutoff)));
>+        rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::impreciseStep)), scratchGPR1);
>+        mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
>+        addPtr(TrustedImmPtr(&subspace.impreciseAllocators[0]), scratchGPR1);
>+
>+        selectedSmallSpace.link(this);
>+        
>+        emitAllocate(resultGPR, scratchGPR1, scratchGPR2, slowPath);
>+    }
>+    
>+    template<typename ClassType, typename StructureType>
>+    void emitAllocateVariableSizedJSObject(GPRReg resultGPR, StructureType structure, GPRReg allocationSize, GPRReg scratchGPR1, GPRReg scratchGPR2, JumpList& slowPath)
>+    {
>+        MarkedSpace::Subspace& subspace = vm()->heap.template subspaceForObjectOfType<ClassType>();
>+        emitAllocateVariableSized(resultGPR, subspace, allocationSize, scratchGPR1, scratchGPR2, slowPath);
>+        emitStoreStructureWithTypeInfo(structure, resultGPR, scratchGPR2);
>+        storePtr(TrustedImmPtr(0), Address(resultGPR, JSObject::butterflyOffset()));
>+    }
>+    
>+    template<typename ClassType>
>+    void emitAllocateDestructibleObject(GPRReg resultGPR, Structure* structure, GPRReg scratchGPR1, GPRReg scratchGPR2, JumpList& slowPath)
>+    {
>+        emitAllocateJSObject<ClassType>(resultGPR, TrustedImmPtr(structure), TrustedImmPtr(0), scratchGPR1, scratchGPR2, slowPath);
>+        storePtr(TrustedImmPtr(structure->classInfo()), Address(resultGPR, JSDestructibleObject::classInfoOffset()));
>+    }
>+    
> protected:
>     VM* m_vm;
>     CodeBlock* m_codeBlock;
>Index: Source/JavaScriptCore/jit/JIT.h
>===================================================================
>--- Source/JavaScriptCore/jit/JIT.h	(revision 203694)
>+++ Source/JavaScriptCore/jit/JIT.h	(working copy)
>@@ -342,9 +342,6 @@ namespace JSC {
>         void emitWriteBarrier(JSCell* owner, unsigned value, WriteBarrierMode);
>         void emitWriteBarrier(JSCell* owner);
> 
>-        template<typename StructureType> // StructureType can be RegisterID or ImmPtr.
>-        void emitAllocateJSObject(RegisterID allocator, StructureType, RegisterID result, RegisterID scratch);
>-        
>         // This assumes that the value to profile is in regT0 and that regT3 is available for
>         // scratch.
>         void emitValueProfilingSite(ValueProfile*);
>Index: Source/JavaScriptCore/jit/JITInlines.h
>===================================================================
>--- Source/JavaScriptCore/jit/JITInlines.h	(revision 203694)
>+++ Source/JavaScriptCore/jit/JITInlines.h	(working copy)
>@@ -941,27 +941,6 @@ ALWAYS_INLINE bool JIT::isOperandConstan
>     return m_codeBlock->isConstantRegisterIndex(src) && getConstantOperand(src).isString() && asString(getConstantOperand(src).asCell())->length() == 1;
> }
> 
>-template<typename StructureType>
>-inline void JIT::emitAllocateJSObject(RegisterID allocator, StructureType structure, RegisterID result, RegisterID scratch)
>-{
>-    if (Options::forceGCSlowPaths())
>-        addSlowCase(jump());
>-    else {
>-        loadPtr(Address(allocator, MarkedAllocator::offsetOfFreeListHead()), result);
>-        addSlowCase(branchTestPtr(Zero, result));
>-    }
>-
>-    // remove the object from the free list
>-    loadPtr(Address(result), scratch);
>-    storePtr(scratch, Address(allocator, MarkedAllocator::offsetOfFreeListHead()));
>-
>-    // initialize the object's property storage pointer
>-    storePtr(TrustedImmPtr(0), Address(result, JSObject::butterflyOffset()));
>-
>-    // initialize the object's structure
>-    emitStoreStructureWithTypeInfo(structure, result, scratch);
>-}
>-
> inline void JIT::emitValueProfilingSite(ValueProfile* valueProfile)
> {
>     ASSERT(shouldEmitProfiling());
>Index: Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JITOpcodes32_64.cpp	(revision 203694)
>+++ Source/JavaScriptCore/jit/JITOpcodes32_64.cpp	(working copy)
>@@ -172,7 +172,9 @@ void JIT::emit_op_new_object(Instruction
>     RegisterID scratchReg = regT3;
> 
>     move(TrustedImmPtr(allocator), allocatorReg);
>-    emitAllocateJSObject(allocatorReg, TrustedImmPtr(structure), resultReg, scratchReg);
>+    JumpList slowCases;
>+    emitAllocateJSObject(resultReg, allocatorReg, TrustedImmPtr(structure), TrustedImmPtr(0), scratchReg, slowCases);
>+    addSlowCase(slowCases);
>     emitStoreCell(currentInstruction[1].u.operand, resultReg);
> }
> 
>@@ -1031,7 +1033,9 @@ void JIT::emit_op_create_this(Instructio
>     addSlowCase(branchPtr(NotEqual, calleeReg, cachedFunctionReg));
>     hasSeenMultipleCallees.link(this);
> 
>-    emitAllocateJSObject(allocatorReg, structureReg, resultReg, scratchReg);
>+    JumpList slowCases;
>+    emitAllocateJSObject(resultReg, allocatorReg, structureReg, TrustedImmPtr(0), scratchReg, slowCases);
>+    addSlowCase(slowCases);
>     emitStoreCell(currentInstruction[1].u.operand, resultReg);
> }
> 
>Index: Source/JavaScriptCore/jit/JITOpcodes.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JITOpcodes.cpp	(revision 203694)
>+++ Source/JavaScriptCore/jit/JITOpcodes.cpp	(working copy)
>@@ -91,7 +91,9 @@ void JIT::emit_op_new_object(Instruction
>     RegisterID scratchReg = regT2;
> 
>     move(TrustedImmPtr(allocator), allocatorReg);
>-    emitAllocateJSObject(allocatorReg, TrustedImmPtr(structure), resultReg, scratchReg);
>+    JumpList slowCases;
>+    emitAllocateJSObject(resultReg, allocatorReg, TrustedImmPtr(structure), TrustedImmPtr(0), scratchReg, slowCases);
>+    addSlowCase(slowCases);
>     emitPutVirtualRegister(currentInstruction[1].u.operand);
> }
> 
>@@ -770,7 +772,9 @@ void JIT::emit_op_create_this(Instructio
>     addSlowCase(branchPtr(NotEqual, calleeReg, cachedFunctionReg));
>     hasSeenMultipleCallees.link(this);
> 
>-    emitAllocateJSObject(allocatorReg, structureReg, resultReg, scratchReg);
>+    JumpList slowCases;
>+    emitAllocateJSObject(resultReg, allocatorReg, structureReg, TrustedImmPtr(0), scratchReg, slowCases);
>+    addSlowCase(slowCases);
>     emitPutVirtualRegister(currentInstruction[1].u.operand);
> }
>

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 160171: 284518 | 284520 | 284521