Attachment #238452 for bug #136992

View Details Raw Unified Return to Bug 136992

| Differences between



2014-09-21  Filip Pizlo  <fpizlo@apple.com>

        FTL should store the four bytes of the cell header using a 32-bit store rather than four 8-bit stores
        https://bugs.webkit.org/show_bug.cgi?id=136992

        Reviewed by NOBODY (OOPS!).
        
        LLVM ought to be able to do this optimization for us given how the code was written, but
        any such lower-level attempts to optimize this would get into trouble with the weird
        object materialization logic I'll be introducing in bug 136330. So, this brings the
        merging of the byte stores into the FTL lowering so that we can control it explicitly.

        * ftl/FTLAbstractHeap.h:
        (JSC::FTL::AbstractHeap::changeParent):
        * ftl/FTLAbstractHeapRepository.cpp:
        (JSC::FTL::AbstractHeapRepository::AbstractHeapRepository):
        * ftl/FTLAbstractHeapRepository.h:
        * ftl/FTLLowerDFGToLLVM.cpp:
        (JSC::FTL::LowerDFGToLLVM::allocateCell):

2014-09-21  Filip Pizlo  <fpizlo@apple.com>

        Structure should have a method for concurrently getting all of the property map entries, and this method shouldn't involve copy-paste

Lines 72-77 public: Source/JavaScriptCore/ftl/FTLAbstractHeap.h_sec1
- Source/JavaScriptCore/ftl/FTLAbstractHeap.h +5 lines
72	m_parent = parent;	72	m_parent = parent;
73	m_heapName = heapName;	73	m_heapName = heapName;
74	}	74	}
		75
		76	void changeParent(AbstractHeap* parent)
		77	{
		78	m_parent = parent;
		79	}
75		80
76	AbstractHeap* parent() const	81	AbstractHeap* parent() const
77	{	82	{



    , m_context(context)
    , m_tbaaKind(mdKindID(m_context, "tbaa"))
{
    // Make sure that our explicit assumptions about the StructureIDBlob match reality.
    RELEASE_ASSERT(!(JSCell_indexingType.offset() & (sizeof(int32_t) - 1)));
    RELEASE_ASSERT(JSCell_indexingType.offset() + 1 == JSCell_typeInfoType.offset());
    RELEASE_ASSERT(JSCell_indexingType.offset() + 2 == JSCell_typeInfoFlags.offset());
    RELEASE_ASSERT(JSCell_indexingType.offset() + 3 == JSCell_gcData.offset());

    JSCell_indexingType.changeParent(&JSCell_usefulBytes);
    JSCell_typeInfoType.changeParent(&JSCell_usefulBytes);
    JSCell_typeInfoFlags.changeParent(&JSCell_usefulBytes);
    JSCell_gcData.changeParent(&JSCell_usefulBytes);
    
    root.m_tbaaMetadata = mdNode(m_context, mdString(m_context, root.m_heapName));
    
    RELEASE_ASSERT(m_tbaaKind);

Lines 52-57 namespace JSC { namespace FTL { Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h_sec1
- Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h +1 lines
52	macro(JSArrayBufferView_mode, JSArrayBufferView::offsetOfMode()) \	52	macro(JSArrayBufferView_mode, JSArrayBufferView::offsetOfMode()) \
53	macro(JSArrayBufferView_vector, JSArrayBufferView::offsetOfVector()) \	53	macro(JSArrayBufferView_vector, JSArrayBufferView::offsetOfVector()) \
54	macro(JSCell_structureID, JSCell::structureIDOffset()) \	54	macro(JSCell_structureID, JSCell::structureIDOffset()) \
		55	macro(JSCell_usefulBytes, JSCell::indexingTypeOffset()) \
55	macro(JSCell_typeInfoFlags, JSCell::typeInfoFlagsOffset()) \	56	macro(JSCell_typeInfoFlags, JSCell::typeInfoFlagsOffset()) \
56	macro(JSCell_typeInfoType, JSCell::typeInfoTypeOffset()) \	57	macro(JSCell_typeInfoType, JSCell::typeInfoTypeOffset()) \
57	macro(JSCell_indexingType, JSCell::indexingTypeOffset()) \	58	macro(JSCell_indexingType, JSCell::indexingTypeOffset()) \



            allocator, m_heaps.MarkedAllocator_freeListHead);
        
        m_out.store32(m_out.constInt32(structure->id()), result, m_heaps.JSCell_structureID);
        
        m_out.store32(
            m_out.constInt32(structure->objectInitializationBlob()),
            result, m_heaps.JSCell_usefulBytes);
        
        return result;
    }

Return to Bug 136992

Lines 4857-4866 private: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp_sec1
- Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp -4 / +4 lines
4857	allocator, m_heaps.MarkedAllocator_freeListHead);	4857	allocator, m_heaps.MarkedAllocator_freeListHead);
4858		4858
4859	m_out.store32(m_out.constInt32(structure->id()), result, m_heaps.JSCell_structureID);	4859	m_out.store32(m_out.constInt32(structure->id()), result, m_heaps.JSCell_structureID);
4860	m_out.store8(m_out.constInt8(structure->indexingType()), result, m_heaps.JSCell_indexingType);	4860
4861	m_out.store8(m_out.constInt8(structure->typeInfo().type()), result, m_heaps.JSCell_typeInfoType);	4861	m_out.store32(
4862	m_out.store8(m_out.constInt8(structure->typeInfo().inlineTypeFlags()), result, m_heaps.JSCell_typeInfoFlags);	4862	m_out.constInt32(structure->objectInitializationBlob()),
4863	m_out.store8(m_out.constInt8(JSCell::NotMarked), result, m_heaps.JSCell_gcData);	4863	result, m_heaps.JSCell_usefulBytes);
4864		4864
4865	return result;	4865	return result;
4866	}	4866	}

Lines 1-3 Source/JavaScriptCore/ChangeLog_sec1
- Source/JavaScriptCore/ChangeLog +20 lines
		1	2014-09-21 Filip Pizlo <fpizlo@apple.com>
		2
		3	FTL should store the four bytes of the cell header using a 32-bit store rather than four 8-bit stores
		4	https://bugs.webkit.org/show_bug.cgi?id=136992
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	LLVM ought to be able to do this optimization for us given how the code was written, but
		9	any such lower-level attempts to optimize this would get into trouble with the weird
		10	object materialization logic I'll be introducing in bug 136330. So, this brings the
		11	merging of the byte stores into the FTL lowering so that we can control it explicitly.
		12
		13	* ftl/FTLAbstractHeap.h:
		14	(JSC::FTL::AbstractHeap::changeParent):
		15	* ftl/FTLAbstractHeapRepository.cpp:
		16	(JSC::FTL::AbstractHeapRepository::AbstractHeapRepository):
		17	* ftl/FTLAbstractHeapRepository.h:
		18	* ftl/FTLLowerDFGToLLVM.cpp:
		19	(JSC::FTL::LowerDFGToLLVM::allocateCell):
		20
1	2014-09-21 Filip Pizlo <fpizlo@apple.com>	21	2014-09-21 Filip Pizlo <fpizlo@apple.com>
2		22
3	Structure should have a method for concurrently getting all of the property map entries, and this method shouldn't involve copy-paste	23	Structure should have a method for concurrently getting all of the property map entries, and this method shouldn't involve copy-paste

Lines 61-66 AbstractHeapRepository::AbstractHeapRepo Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp_sec1
- Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.cpp +11 lines
61	, m_context(context)	61	, m_context(context)
62	, m_tbaaKind(mdKindID(m_context, "tbaa"))	62	, m_tbaaKind(mdKindID(m_context, "tbaa"))
63	{	63	{
		64	// Make sure that our explicit assumptions about the StructureIDBlob match reality.
		65	RELEASE_ASSERT(!(JSCell_indexingType.offset() & (sizeof(int32_t) - 1)));
		66	RELEASE_ASSERT(JSCell_indexingType.offset() + 1 == JSCell_typeInfoType.offset());
		67	RELEASE_ASSERT(JSCell_indexingType.offset() + 2 == JSCell_typeInfoFlags.offset());
		68	RELEASE_ASSERT(JSCell_indexingType.offset() + 3 == JSCell_gcData.offset());
		69
		70	JSCell_indexingType.changeParent(&JSCell_usefulBytes);
		71	JSCell_typeInfoType.changeParent(&JSCell_usefulBytes);
		72	JSCell_typeInfoFlags.changeParent(&JSCell_usefulBytes);
		73	JSCell_gcData.changeParent(&JSCell_usefulBytes);
		74
64	root.m_tbaaMetadata = mdNode(m_context, mdString(m_context, root.m_heapName));	75	root.m_tbaaMetadata = mdNode(m_context, mdString(m_context, root.m_heapName));
65		76
66	RELEASE_ASSERT(m_tbaaKind);	77	RELEASE_ASSERT(m_tbaaKind);