InlineAccess should do StringLength
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2018 19:46:56 +0000 (19:46 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2018 19:46:56 +0000 (19:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158911

Reviewed by Yusuke Suzuki.

This patch extends InlineAccess to support StringLength. This patch also
fixes AccessCase::fromStructureStubInfo to support ArrayLength and StringLength.
I forgot to implement this for ArrayLength in the initial InlineAccess
implementation.  Supporting StringLength is a natural extension of the
InlineAccess machinery.

* assembler/MacroAssembler.h:
(JSC::MacroAssembler::patchableBranch8):
* assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::patchableBranch8):
* bytecode/AccessCase.cpp:
(JSC::AccessCase::fromStructureStubInfo):
* bytecode/BytecodeDumper.cpp:
(JSC::BytecodeDumper<Block>::printGetByIdCacheStatus):
* bytecode/InlineAccess.cpp:
(JSC::InlineAccess::dumpCacheSizesAndCrash):
(JSC::InlineAccess::generateSelfPropertyAccess):
(JSC::getScratchRegister):
(JSC::InlineAccess::generateSelfPropertyReplace):
(JSC::InlineAccess::generateArrayLength):
(JSC::InlineAccess::generateSelfInAccess):
(JSC::InlineAccess::generateStringLength):
* bytecode/InlineAccess.h:
* bytecode/PolymorphicAccess.cpp:
(JSC::PolymorphicAccess::regenerate):
* bytecode/StructureStubInfo.cpp:
(JSC::StructureStubInfo::initStringLength):
(JSC::StructureStubInfo::deref):
(JSC::StructureStubInfo::aboutToDie):
(JSC::StructureStubInfo::propagateTransitions):
* bytecode/StructureStubInfo.h:
(JSC::StructureStubInfo::baseGPR const):
* jit/Repatch.cpp:
(JSC::tryCacheGetByID):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MacroAssembler.h
Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
Source/JavaScriptCore/bytecode/AccessCase.cpp
Source/JavaScriptCore/bytecode/BytecodeDumper.cpp
Source/JavaScriptCore/bytecode/InlineAccess.cpp
Source/JavaScriptCore/bytecode/InlineAccess.h
Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp
Source/JavaScriptCore/bytecode/StructureStubInfo.cpp
Source/JavaScriptCore/bytecode/StructureStubInfo.h
Source/JavaScriptCore/jit/Repatch.cpp

index 95c8361..83f8565 100644 (file)
@@ -1,5 +1,47 @@
 2018-08-30  Saam barati  <sbarati@apple.com>
 
+        InlineAccess should do StringLength
+        https://bugs.webkit.org/show_bug.cgi?id=158911
+
+        Reviewed by Yusuke Suzuki.
+
+        This patch extends InlineAccess to support StringLength. This patch also
+        fixes AccessCase::fromStructureStubInfo to support ArrayLength and StringLength.
+        I forgot to implement this for ArrayLength in the initial InlineAccess
+        implementation.  Supporting StringLength is a natural extension of the
+        InlineAccess machinery.
+
+        * assembler/MacroAssembler.h:
+        (JSC::MacroAssembler::patchableBranch8):
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::patchableBranch8):
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::fromStructureStubInfo):
+        * bytecode/BytecodeDumper.cpp:
+        (JSC::BytecodeDumper<Block>::printGetByIdCacheStatus):
+        * bytecode/InlineAccess.cpp:
+        (JSC::InlineAccess::dumpCacheSizesAndCrash):
+        (JSC::InlineAccess::generateSelfPropertyAccess):
+        (JSC::getScratchRegister):
+        (JSC::InlineAccess::generateSelfPropertyReplace):
+        (JSC::InlineAccess::generateArrayLength):
+        (JSC::InlineAccess::generateSelfInAccess):
+        (JSC::InlineAccess::generateStringLength):
+        * bytecode/InlineAccess.h:
+        * bytecode/PolymorphicAccess.cpp:
+        (JSC::PolymorphicAccess::regenerate):
+        * bytecode/StructureStubInfo.cpp:
+        (JSC::StructureStubInfo::initStringLength):
+        (JSC::StructureStubInfo::deref):
+        (JSC::StructureStubInfo::aboutToDie):
+        (JSC::StructureStubInfo::propagateTransitions):
+        * bytecode/StructureStubInfo.h:
+        (JSC::StructureStubInfo::baseGPR const):
+        * jit/Repatch.cpp:
+        (JSC::tryCacheGetByID):
+
+2018-08-30  Saam barati  <sbarati@apple.com>
+
         CSE DataViewGet* DFG nodes
         https://bugs.webkit.org/show_bug.cgi?id=188768
 
index a1edb9b..e89cb14 100644 (file)
@@ -450,6 +450,11 @@ public:
         return PatchableJump(branch32(cond, reg, imm));
     }
 
+    PatchableJump patchableBranch8(RelationalCondition cond, Address address, TrustedImm32 imm)
+    {
+        return PatchableJump(branch8(cond, address, imm));
+    }
+
     PatchableJump patchableBranch32(RelationalCondition cond, Address address, TrustedImm32 imm)
     {
         return PatchableJump(branch32(cond, address, imm));
index 42d0884..a7af254 100644 (file)
@@ -3388,6 +3388,14 @@ public:
         return PatchableJump(result);
     }
 
+    PatchableJump patchableBranch8(RelationalCondition cond, Address left, TrustedImm32 imm)
+    {
+        m_makeJumpPatchable = true;
+        Jump result = branch8(cond, left, imm);
+        m_makeJumpPatchable = false;
+        return PatchableJump(result);
+    }
+
     PatchableJump patchableBranchTest32(ResultCondition cond, RegisterID reg, TrustedImm32 mask = TrustedImm32(-1))
     {
         m_makeJumpPatchable = true;
index f36c1fe..7daf85a 100644 (file)
@@ -121,6 +121,12 @@ std::unique_ptr<AccessCase> AccessCase::fromStructureStubInfo(
     case CacheType::InByIdSelf:
         return AccessCase::create(vm, owner, InHit, stubInfo.u.byIdSelf.offset, stubInfo.u.byIdSelf.baseObjectStructure.get());
 
+    case CacheType::ArrayLength:
+        return AccessCase::create(vm, owner, AccessCase::ArrayLength);
+
+    case CacheType::StringLength:
+        return AccessCase::create(vm, owner, AccessCase::StringLength);
+
     default:
         return nullptr;
     }
index 6ec0620..70bf712 100644 (file)
@@ -447,6 +447,9 @@ void BytecodeDumper<Block>::printGetByIdCacheStatus(PrintStream& out, int locati
         case CacheType::ArrayLength:
             out.printf("ArrayLength");
             break;
+        case CacheType::StringLength:
+            out.printf("StringLength");
+            break;
         default:
             RELEASE_ASSERT_NOT_REACHED();
             break;
index bd86fa3..86ec29b 100644 (file)
@@ -47,6 +47,18 @@ void InlineAccess::dumpCacheSizesAndCrash()
 #else
     JSValueRegs regs(base);
 #endif
+    {
+        CCallHelpers jit;
+
+        jit.patchableBranch8(
+            CCallHelpers::NotEqual,
+            CCallHelpers::Address(base, JSCell::typeInfoTypeOffset()),
+            CCallHelpers::TrustedImm32(StringType));
+        jit.load32(CCallHelpers::Address(base, JSString::offsetOfLength()), regs.payloadGPR());
+        jit.boxInt32(regs.payloadGPR(), regs);
+
+        dataLog("string length size: ", jit.m_assembler.buffer().codeSize(), "\n");
+    }
 
     {
         CCallHelpers jit;
@@ -158,7 +170,7 @@ bool InlineAccess::generateSelfPropertyAccess(StructureStubInfo& stubInfo, Struc
 {
     CCallHelpers jit;
     
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
 
     auto branchToSlowPath = jit.patchableBranch32(
@@ -185,7 +197,7 @@ bool InlineAccess::generateSelfPropertyAccess(StructureStubInfo& stubInfo, Struc
 ALWAYS_INLINE static GPRReg getScratchRegister(StructureStubInfo& stubInfo)
 {
     ScratchRegisterAllocator allocator(stubInfo.patch.usedRegisters);
-    allocator.lock(static_cast<GPRReg>(stubInfo.patch.baseGPR));
+    allocator.lock(stubInfo.baseGPR());
     allocator.lock(static_cast<GPRReg>(stubInfo.patch.valueGPR));
 #if USE(JSVALUE32_64)
     allocator.lock(static_cast<GPRReg>(stubInfo.patch.baseTagGPR));
@@ -216,7 +228,7 @@ bool InlineAccess::generateSelfPropertyReplace(StructureStubInfo& stubInfo, Stru
 
     CCallHelpers jit;
 
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
 
     auto branchToSlowPath = jit.patchableBranch32(
@@ -258,7 +270,7 @@ bool InlineAccess::generateArrayLength(StructureStubInfo& stubInfo, JSArray* arr
 
     CCallHelpers jit;
 
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
     GPRReg scratch = getScratchRegister(stubInfo);
 
@@ -276,11 +288,32 @@ bool InlineAccess::generateArrayLength(StructureStubInfo& stubInfo, JSArray* arr
     return linkedCodeInline;
 }
 
+bool InlineAccess::generateStringLength(StructureStubInfo& stubInfo)
+{
+    CCallHelpers jit;
+
+    GPRReg base = stubInfo.baseGPR();
+    JSValueRegs value = stubInfo.valueRegs();
+
+    auto branchToSlowPath = jit.patchableBranch8(
+        CCallHelpers::NotEqual,
+        CCallHelpers::Address(base, JSCell::typeInfoTypeOffset()),
+        CCallHelpers::TrustedImm32(StringType));
+    jit.load32(CCallHelpers::Address(base, JSString::offsetOfLength()), value.payloadGPR());
+    jit.boxInt32(value.payloadGPR(), value);
+
+    bool linkedCodeInline = linkCodeInline("string length", jit, stubInfo, [&] (LinkBuffer& linkBuffer) {
+        linkBuffer.link(branchToSlowPath, stubInfo.slowPathStartLocation());
+    });
+    return linkedCodeInline;
+}
+
+
 bool InlineAccess::generateSelfInAccess(StructureStubInfo& stubInfo, Structure* structure)
 {
     CCallHelpers jit;
 
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
 
     auto branchToSlowPath = jit.patchableBranch32(
index a4d0335..8d0846a 100644 (file)
@@ -87,7 +87,7 @@ public:
     // FIXME: Make this constexpr when GCC is able to compile std::max() inside a constexpr function.
     // https://bugs.webkit.org/show_bug.cgi?id=159436
     //
-    // This is the maximum between the size for array length access, and the size for regular self access.
+    // This is the maximum between array length, string length, and regular self access sizes.
     ALWAYS_INLINE static size_t sizeForLengthAccess()
     {
 #if CPU(X86_64)
@@ -117,6 +117,7 @@ public:
     static bool generateArrayLength(StructureStubInfo&, JSArray*);
     static void rewireStubAsJump(StructureStubInfo&, CodeLocationLabel<JITStubRoutinePtrTag>);
     static bool generateSelfInAccess(StructureStubInfo&, Structure*);
+    static bool generateStringLength(StructureStubInfo&);
 
     // This is helpful when determining the size of an IC on
     // various platforms. When adding a new type of IC, implement
index 585fcb5..fc44d4e 100644 (file)
@@ -381,7 +381,7 @@ AccessGenerationResult PolymorphicAccess::regenerate(
     state.stubInfo = &stubInfo;
     state.ident = &ident;
     
-    state.baseGPR = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    state.baseGPR = stubInfo.baseGPR();
     state.thisGPR = static_cast<GPRReg>(stubInfo.patch.thisGPR);
     state.valueRegs = stubInfo.valueRegs();
 
index db200d9..354ccf4 100644 (file)
@@ -73,6 +73,11 @@ void StructureStubInfo::initArrayLength()
     cacheType = CacheType::ArrayLength;
 }
 
+void StructureStubInfo::initStringLength()
+{
+    cacheType = CacheType::StringLength;
+}
+
 void StructureStubInfo::initPutByIdReplace(CodeBlock* codeBlock, Structure* baseObjectStructure, PropertyOffset offset)
 {
     cacheType = CacheType::PutByIdReplace;
@@ -102,6 +107,7 @@ void StructureStubInfo::deref()
     case CacheType::PutByIdReplace:
     case CacheType::InByIdSelf:
     case CacheType::ArrayLength:
+    case CacheType::StringLength:
         return;
     }
 
@@ -119,6 +125,7 @@ void StructureStubInfo::aboutToDie()
     case CacheType::PutByIdReplace:
     case CacheType::InByIdSelf:
     case CacheType::ArrayLength:
+    case CacheType::StringLength:
         return;
     }
 
@@ -292,6 +299,7 @@ bool StructureStubInfo::propagateTransitions(SlotVisitor& visitor)
     switch (cacheType) {
     case CacheType::Unset:
     case CacheType::ArrayLength:
+    case CacheType::StringLength:
         return true;
     case CacheType::GetByIdSelf:
     case CacheType::PutByIdReplace:
index 6d03355..170826d 100644 (file)
@@ -61,7 +61,8 @@ enum class CacheType : int8_t {
     PutByIdReplace,
     InByIdSelf,
     Stub,
-    ArrayLength
+    ArrayLength,
+    StringLength
 };
 
 class StructureStubInfo {
@@ -73,6 +74,7 @@ public:
 
     void initGetByIdSelf(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
     void initArrayLength();
+    void initStringLength();
     void initPutByIdReplace(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
     void initInByIdSelf(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
 
@@ -199,6 +201,11 @@ public:
 #endif
     } patch;
 
+    GPRReg baseGPR() const
+    {
+        return static_cast<GPRReg>(patch.baseGPR);
+    }
+
     CodeLocationCall<JSInternalPtrTag> slowPathCallLocation() { return patch.start.callAtOffset<JSInternalPtrTag>(patch.deltaFromStartToSlowPathCallLocation); }
     CodeLocationLabel<JSInternalPtrTag> doneLocation() { return patch.start.labelAtOffset<JSInternalPtrTag>(patch.inlineSize); }
     CodeLocationLabel<JITStubRoutinePtrTag> slowPathStartLocation() { return patch.start.labelAtOffset(patch.deltaFromStartToSlowPathStart); }
index c08e419..6686df0 100644 (file)
@@ -215,8 +215,18 @@ static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, con
                 }
 
                 newCase = AccessCase::create(vm, codeBlock, AccessCase::ArrayLength);
-            } else if (isJSString(baseCell))
+            } else if (isJSString(baseCell)) {
+                if (stubInfo.cacheType == CacheType::Unset) {
+                    bool generatedCodeInline = InlineAccess::generateStringLength(stubInfo);
+                    if (generatedCodeInline) {
+                        ftlThunkAwareRepatchCall(codeBlock, stubInfo.slowPathCallLocation(), appropriateOptimizingGetByIdFunction(kind));
+                        stubInfo.initStringLength();
+                        return RetryCacheLater;
+                    }
+                }
+
                 newCase = AccessCase::create(vm, codeBlock, AccessCase::StringLength);
+            }
             else if (DirectArguments* arguments = jsDynamicCast<DirectArguments*>(vm, baseCell)) {
                 // If there were overrides, then we can handle this as a normal property load! Guarding
                 // this with such a check enables us to add an IC case for that load if needed.