Changeset 235517 in webkit

Timestamp:

Aug 30, 2018 12:46:56 PM (6 years ago)

Author:

sbarati@apple.com

Message:

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssembler.h (modified) (1 diff)
• assembler/MacroAssemblerARM64.h (modified) (1 diff)
• bytecode/AccessCase.cpp (modified) (1 diff)
• bytecode/BytecodeDumper.cpp (modified) (1 diff)
• bytecode/InlineAccess.cpp (modified) (6 diffs)
• bytecode/InlineAccess.h (modified) (2 diffs)
• bytecode/PolymorphicAccess.cpp (modified) (1 diff)
• bytecode/StructureStubInfo.cpp (modified) (4 diffs)
• bytecode/StructureStubInfo.h (modified) (3 diffs)
• jit/Repatch.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/assembler/MacroAssembler.h

@@ -449 +449 @@
     {
         return PatchableJump(branch32(cond, reg, imm));
+    }
+
+    PatchableJump patchableBranch8(RelationalCondition cond, Address address, TrustedImm32 imm)
+    {
+        return PatchableJump(branch8(cond, address, imm));
     }
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -3389 +3389 @@
     }
 
+    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))
     {

trunk/Source/JavaScriptCore/bytecode/AccessCase.cpp

@@ -121 +121 @@
     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:

trunk/Source/JavaScriptCore/bytecode/BytecodeDumper.cpp

@@ -447 +447 @@
         case CacheType::ArrayLength:
             out.printf("ArrayLength");
+            break;
+        case CacheType::StringLength:
+            out.printf("StringLength");
             break;
         default:

trunk/Source/JavaScriptCore/bytecode/InlineAccess.cpp

@@ -48 +48 @@
     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");
+    }
 
     {
@@ -159 +171 @@
     CCallHelpers jit;
     
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
 
@@ -186 +198 @@
 {
     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)
@@ -217 +229 @@
     CCallHelpers jit;
 
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
 
@@ -259 +271 @@
     CCallHelpers jit;
 
-    GPRReg base = static_cast<GPRReg>(stubInfo.patch.baseGPR);
+    GPRReg base = stubInfo.baseGPR();
     JSValueRegs value = stubInfo.valueRegs();
     GPRReg scratch = getScratchRegister(stubInfo);
@@ -277 +289 @@
 }
 
+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();
 

trunk/Source/JavaScriptCore/bytecode/InlineAccess.h

@@ -88 +88 @@
     // 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()
     {
@@ -118 +118 @@
     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

trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp

@@ -382 +382 @@
     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();

trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp

@@ -74 +74 @@
 }
 
+void StructureStubInfo::initStringLength()
+{
+    cacheType = CacheType::StringLength;
+}
+
 void StructureStubInfo::initPutByIdReplace(CodeBlock* codeBlock, Structure* baseObjectStructure, PropertyOffset offset)
 {
@@ -103 +108 @@
     case CacheType::InByIdSelf:
     case CacheType::ArrayLength:
+    case CacheType::StringLength:
         return;
     }
@@ -120 +126 @@
     case CacheType::InByIdSelf:
     case CacheType::ArrayLength:
+    case CacheType::StringLength:
         return;
     }
@@ -293 +300 @@
     case CacheType::Unset:
     case CacheType::ArrayLength:
+    case CacheType::StringLength:
         return true;
     case CacheType::GetByIdSelf:

trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h

@@ -62 +62 @@
     InByIdSelf,
     Stub,
-    ArrayLength
+    ArrayLength,
+    StringLength
 };
 
@@ -74 +75 @@
     void initGetByIdSelf(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
     void initArrayLength();
+    void initStringLength();
     void initPutByIdReplace(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
     void initInByIdSelf(CodeBlock*, Structure* baseObjectStructure, PropertyOffset);
@@ -200 +202 @@
     } 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); }

trunk/Source/JavaScriptCore/jit/Repatch.cpp

@@ -216 +216 @@
 
                 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