Changeset 293710 in webkit

Timestamp:

May 2, 2022, 8:02:04 PM (4 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Introduce shifting Structure encoding
​https://bugs.webkit.org/show_bug.cgi?id=239957

Reviewed by Mark Lam.

For platforms which have limited amount of virtual address space (<= 36 bits), this patch introduces
shifting Structure encoding. We align Structure on a 32-bytes boundary instead of 16 bytes so that
we can ensure that lower 5 bits are zero. Then, we can use 1 bit for nuke, and shifting 4 bits to
convert 36 bit address to 32 bit StructureID. By using this mechanism, we do not need to allocate
large virtual address space for these platforms. If we an address can have more than 36 bits, then
we should just reserve a larger address region since we have enough address space. Current Structure
size is 112 bytes, which is 3.5 atoms at 32 bytes / atom. Hence, this alignment costs us 16 bytes per
Structure.
Relanding with debug build failure & crash on static atomSize assumption in IsoSubspace.

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):

• heap/Heap.cpp:
• heap/IsoSubspace.h:

(JSC::GCClient::IsoSubspace::allocatorFor):

• heap/IsoSubspaceInlines.h:

(JSC::GCClient::IsoSubspace::allocate):

• heap/StructureAlignedMemoryAllocator.cpp:

(JSC::StructureMemoryManager::StructureMemoryManager):

• jit/AssemblyHelpers.cpp:

(JSC::AssemblyHelpers::emitNonNullDecodeStructureID):

• llint/LLIntOfflineAsmConfig.h:
• llint/LowLevelInterpreter64.asm:
• runtime/JSCell.h:
• runtime/JSCellInlines.h:

(JSC::JSCell::JSCell):

• runtime/Structure.cpp:

(JSC::Structure::Structure):

• runtime/Structure.h:
• runtime/StructureID.h:

(JSC::StructureID::decode const):
(JSC::StructureID::tryDecode const):
(JSC::StructureID::encode):

• tools/IntegrityInlines.h:

(JSC::Integrity::auditStructureID):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• ftl/FTLLowerDFGToB3.cpp (modified) (1 diff)
• heap/Heap.cpp (modified) (1 diff)
• heap/IsoSubspace.h (modified) (1 diff)
• heap/IsoSubspaceInlines.h (modified) (1 diff)
• heap/StructureAlignedMemoryAllocator.cpp (modified) (2 diffs)
• jit/AssemblyHelpers.cpp (modified) (1 diff)
• llint/LLIntOfflineAsmConfig.h (modified) (2 diffs)
• llint/LowLevelInterpreter64.asm (modified) (1 diff)
• runtime/JSCell.h (modified) (1 diff)
• runtime/JSCellInlines.h (modified) (1 diff)
• runtime/Structure.cpp (modified) (3 diffs)
• runtime/Structure.h (modified) (1 diff)
• runtime/StructureID.h (modified) (2 diffs)
• tools/IntegrityInlines.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2022-05-02  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Introduce shifting Structure encoding
+        https://bugs.webkit.org/show_bug.cgi?id=239957
+
+        Reviewed by Mark Lam.
+
+        For platforms which have limited amount of virtual address space (<= 36 bits), this patch introduces
+        shifting Structure encoding. We align Structure on a 32-bytes boundary instead of 16 bytes so that
+        we can ensure that lower 5 bits are zero. Then, we can use 1 bit for nuke, and shifting 4 bits to
+        convert 36 bit address to 32 bit StructureID. By using this mechanism, we do not need to allocate
+        large virtual address space for these platforms. If we an address can have more than 36 bits, then
+        we should just reserve a larger address region since we have enough address space. Current Structure
+        size is 112 bytes, which is 3.5 atoms at 32 bytes / atom. Hence, this alignment costs us 16 bytes per
+        Structure.
+        Relanding with debug build failure & crash on static atomSize assumption in IsoSubspace.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+        * heap/Heap.cpp:
+        * heap/IsoSubspace.h:
+        (JSC::GCClient::IsoSubspace::allocatorFor):
+        * heap/IsoSubspaceInlines.h:
+        (JSC::GCClient::IsoSubspace::allocate):
+        * heap/StructureAlignedMemoryAllocator.cpp:
+        (JSC::StructureMemoryManager::StructureMemoryManager):
+        * jit/AssemblyHelpers.cpp:
+        (JSC::AssemblyHelpers::emitNonNullDecodeStructureID):
+        * llint/LLIntOfflineAsmConfig.h:
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/JSCell.h:
+        * runtime/JSCellInlines.h:
+        (JSC::JSCell::JSCell):
+        * runtime/Structure.cpp:
+        (JSC::Structure::Structure):
+        * runtime/Structure.h:
+        * runtime/StructureID.h:
+        (JSC::StructureID::decode const):
+        (JSC::StructureID::tryDecode const):
+        (JSC::StructureID::encode):
+        * tools/IntegrityInlines.h:
+        (JSC::Integrity::auditStructureID):
+
 2022-05-01  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -21015 +21015 @@
     LValue decodeNonNullStructure(LValue structureID)
     {
-        LValue maskedStructureID = m_out.bitAnd(structureID, m_out.constInt32(structureIDMask));
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+        return m_out.shl(m_out.zeroExtPtr(structureID), m_out.constIntPtr(StructureID::encodeShiftAmount));
+#else
+        LValue maskedStructureID = m_out.bitAnd(structureID, m_out.constInt32(StructureID::structureIDMask));
         return m_out.add(m_out.constIntPtr(g_jscConfig.startOfStructureHeap), m_out.zeroExtPtr(maskedStructureID));
+#endif
     }
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -270 +270 @@
 
 #define INIT_SERVER_STRUCTURE_ISO_SUBSPACE(name, heapCellType, type) \
-    , name("Isolated" #name "Space", *this, heapCellType, sizeof(type), type::numberOfLowerTierCells, makeUnique<StructureAlignedMemoryAllocator>("Structure"))
+    , name("Isolated" #name "Space", *this, heapCellType, WTF::roundUpToMultipleOf<type::atomSize>(sizeof(type)), type::numberOfLowerTierCells, makeUnique<StructureAlignedMemoryAllocator>("Structure"))
 
 Heap::Heap(VM& vm, HeapType heapType)

trunk/Source/JavaScriptCore/heap/IsoSubspace.h

@@ -93 +93 @@
 ALWAYS_INLINE Allocator IsoSubspace::allocatorFor(size_t size, AllocatorForMode)
 {
-    RELEASE_ASSERT(WTF::roundUpToMultipleOf<MarkedBlock::atomSize>(size) == cellSize());
+    RELEASE_ASSERT(size <= cellSize());
     return Allocator(&m_localAllocator);
 }

trunk/Source/JavaScriptCore/heap/IsoSubspaceInlines.h

@@ -32 +32 @@
 ALWAYS_INLINE void* IsoSubspace::allocate(VM& vm, size_t size, GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
 {
-    RELEASE_ASSERT(WTF::roundUpToMultipleOf<MarkedBlock::atomSize>(size) == cellSize());
+    RELEASE_ASSERT(size <= cellSize());
     Allocator allocator = allocatorFor(size, AllocatorForMode::MustAlreadyHaveAllocator);
     void* result = allocator.allocate(vm.heap, deferralContext, failureMode);

trunk/Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp

@@ -67 +67 @@
 }
 
-#if CPU(ADDRESS64)
+#if CPU(ADDRESS64) && !ENABLE(STRUCTURE_ID_WITH_SHIFT)
 
 class StructureMemoryManager {
@@ -84 +84 @@
         }
         g_jscConfig.sizeOfStructureHeap = mappedHeapSize;
-        RELEASE_ASSERT(g_jscConfig.startOfStructureHeap && ((g_jscConfig.startOfStructureHeap & ~structureIDMask) == g_jscConfig.startOfStructureHeap));
+        RELEASE_ASSERT(g_jscConfig.startOfStructureHeap && ((g_jscConfig.startOfStructureHeap & ~StructureID::structureIDMask) == g_jscConfig.startOfStructureHeap));
     }
 

trunk/Source/JavaScriptCore/jit/AssemblyHelpers.cpp

@@ -406 +406 @@
 void AssemblyHelpers::emitNonNullDecodeStructureID(RegisterID source, RegisterID dest)
 {
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+    lshift64(source, TrustedImm32(StructureID::encodeShiftAmount), dest);
+#elif CPU(ADDRESS64)
+    // This could use BFI on arm64 but that only helps if the start of structure heap is encodable as a mov and not as an immediate in the add so it's probably not super important.
+    and32(TrustedImm32(StructureID::structureIDMask), source, dest);
+    add64(TrustedImm64(g_jscConfig.startOfStructureHeap), dest);
+#else // not CPU(ADDRESS64)
     move(source, dest);
-#if CPU(ADDRESS64)
-    // This could use BFI on arm64 but that only helps if the start of structure heap is encodable as a mov and not as an immediate in the add so it's probably not super important.
-    and32(TrustedImm32(structureIDMask), dest);
-    add64(TrustedImm64(g_jscConfig.startOfStructureHeap), dest);
-#endif // not CPU(ADDRESS64)
+#endif
 }
 

trunk/Source/JavaScriptCore/llint/LLIntOfflineAsmConfig.h

@@ -27 +27 @@
 
 #include "LLIntCommon.h"
+#include "StructureID.h"
 #include <wtf/Assertions.h>
 #include <wtf/Gigacage.h>
@@ -162 +163 @@
 #endif
 
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+#define OFFLINE_ASM_STRUCTURE_ID_WITH_SHIFT 1
+#else
+#define OFFLINE_ASM_STRUCTURE_ID_WITH_SHIFT 0
+#endif
+
 #if ASSERT_ENABLED
 #define OFFLINE_ASM_ASSERT_ENABLED 1

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

@@ -742 +742 @@
 
 macro structureIDToStructureWithScratch(structureIDThenStructure, scratch)
-    if ADDRESS64
-        andq constexpr structureIDMask, structureIDThenStructure
+    if STRUCTURE_ID_WITH_SHIFT
+        lshiftp (constexpr StructureID::encodeShiftAmount), structureIDThenStructure
+    elsif ADDRESS64
+        andq (constexpr StructureID::structureIDMask), structureIDThenStructure
         leap JSCConfig + constexpr JSC::offsetOfJSCConfigStartOfStructureHeap, scratch
         loadp [scratch], scratch

trunk/Source/JavaScriptCore/runtime/JSCell.h

@@ -90 +90 @@
     static constexpr uint8_t numberOfLowerTierCells = 8;
 
+    static constexpr size_t atomSize = 16; // This needs to be larger or equal to 16.
+
     static JSCell* seenMultipleCalleeObjects() { return bitwise_cast<JSCell*>(static_cast<uintptr_t>(1)); }
 

trunk/Source/JavaScriptCore/runtime/JSCellInlines.h

@@ -76 +76 @@
     // structure pointer is still alive at this point.
     ensureStillAliveHere(structure);
+    static_assert(JSCell::atomSize >= MarkedBlock::atomSize);
 }
 

trunk/Source/JavaScriptCore/runtime/Structure.cpp

@@ -233 +233 @@
 
     validateFlags();
+
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+    ASSERT(WTF::roundUpToMultipleOf<Structure::atomSize>(this) == this);
+#endif
 }
 
@@ -271 +275 @@
     ASSERT(hasGetterSetterProperties() || !m_classInfo->hasStaticSetterOrReadonlyProperties());
     ASSERT(!this->typeInfo().overridesGetCallData() || m_classInfo->methodTable.getCallData != &JSCell::getCallData);
+
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+    ASSERT(WTF::roundUpToMultipleOf<Structure::atomSize>(this) == this);
+#endif
 }
 
@@ -320 +328 @@
     ASSERT(hasGetterSetterProperties() || !m_classInfo->hasStaticSetterOrReadonlyProperties());
     ASSERT(!this->typeInfo().overridesGetCallData() || m_classInfo->methodTable.getCallData != &JSCell::getCallData);
+
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+    ASSERT(WTF::roundUpToMultipleOf<Structure::atomSize>(this) == this);
+#endif
 }
 

trunk/Source/JavaScriptCore/runtime/Structure.h

@@ -175 +175 @@
     static constexpr unsigned StructureFlags = Base::StructureFlags | StructureIsImmortal;
     static constexpr uint8_t numberOfLowerTierCells = 0;
-    
+
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+    static constexpr size_t atomSize = 32;
+#endif
+    static_assert(JSCell::atomSize >= MarkedBlock::atomSize);
+
     enum PolyProtoTag { PolyProto };
     static Structure* create(VM&, JSGlobalObject*, JSValue prototype, const TypeInfo&, const ClassInfo*, IndexingType = NonArray, unsigned inlineCapacity = 0);

trunk/Source/JavaScriptCore/runtime/StructureID.h

@@ -35 +35 @@
 class Structure;
 
-constexpr CPURegister structureIDMask = structureHeapAddressSize - 1;
+// We would like to define this value in PlatformEnable.h, but it is not possible since the following is relying on MACH_VM_MAX_ADDRESS.
+#if CPU(ADDRESS64) && CPU(ARM64) && OS(DARWIN)
+#if MACH_VM_MAX_ADDRESS_RAW < (1ULL << 36)
+#define ENABLE_STRUCTURE_ID_WITH_SHIFT 1
+static_assert(MACH_VM_MAX_ADDRESS_RAW == MACH_VM_MAX_ADDRESS);
+#endif
+#endif
 
 class StructureID {
 public:
     static constexpr uint32_t nukedStructureIDBit = 1;
+
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+    // ENABLE(STRUCTURE_ID_WITH_SHIFT) is used when our virtual memory space is limited (specifically, less than or equal to 36 bit) while pointer is 64 bit.
+    // In that case, we round up Structures size with 32 bytes instead of 16 bytes. This ensures that lower 5 bit become zero for Structure.
+    // By shifting this address with 4, we can encode 36 bit address into 32 bit StructureID. And we can ensure that StructureID's lowest bit is still zero
+    // because we round Structure size with 32 bytes. This lowest bit is used for nuke bit.
+    static constexpr unsigned encodeShiftAmount = 4;
+#elif CPU(ADDRESS64)
+    static constexpr CPURegister structureIDMask = structureHeapAddressSize - 1;
+#endif
 
     StructureID() = default;
@@ -68 +84 @@
 static_assert(sizeof(StructureID) == sizeof(uint32_t));
 
-#if CPU(ADDRESS64)
+#if ENABLE(STRUCTURE_ID_WITH_SHIFT)
+
+ALWAYS_INLINE Structure* StructureID::decode() const
+{
+    ASSERT(decontaminate());
+    return reinterpret_cast<Structure*>(static_cast<uintptr_t>(m_bits) << encodeShiftAmount);
+}
+
+ALWAYS_INLINE Structure* StructureID::tryDecode() const
+{
+    // Take care to only use the bits from m_bits in the structure's address reservation.
+    uintptr_t address = static_cast<uintptr_t>(decontaminate().m_bits) << encodeShiftAmount;
+    if (address < MarkedBlock::blockSize)
+        return nullptr;
+    return reinterpret_cast<Structure*>(address);
+}
+
+ALWAYS_INLINE StructureID StructureID::encode(const Structure* structure)
+{
+    ASSERT(structure);
+    auto result = StructureID(reinterpret_cast<uintptr_t>(structure) >> encodeShiftAmount);
+    ASSERT(result.decode() == structure);
+    return result;
+}
+
+#elif CPU(ADDRESS64)
 
 ALWAYS_INLINE Structure* StructureID::decode() const

trunk/Source/JavaScriptCore/tools/IntegrityInlines.h

@@ -78 +78 @@
 {
     UNUSED_PARAM(structureID);
-#if CPU(ADDRESS64)
+#if CPU(ADDRESS64) && !ENABLE(STRUCTURE_ID_WITH_SHIFT)
     ASSERT(structureID.bits() <= structureHeapAddressSize + StructureID::nukedStructureIDBit);
 #endif