Attachment 340821 Details for Bug 185810 – Patch

[patch] Patch

bug-185810-20180521133954.patch (text/plain), 15.27 KB, created by Yusuke Suzuki on 2018-05-20 21:39:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-20 21:39:55 PDT

Size: 15.27 KB

patch

obsolete

>Subversion Revision: 232008
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 399fa28df17e881089edecf0312efc81418d65f8..3a1111a08e1858b14ec1ba09546c693f6fdcd73c 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-05-20  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [JSC] Use branchIfString/branchIfNotString instead of structure checkings
>+        https://bugs.webkit.org/show_bug.cgi?id=185810
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Let's use branchIfString/branchIfNotString helper functions instead of
>+        checking structure with jsString's structure. It's easy to read. And
>+        it emits less code since we do not need to embed string structure's
>+        raw pointer.
>+
>+        * jit/JIT.h:
>+        * jit/JITInlines.h:
>+        (JSC::JIT::emitLoadCharacterString):
>+        (JSC::JIT::checkStructure): Deleted.
>+        * jit/JITOpcodes32_64.cpp:
>+        (JSC::JIT::emitSlow_op_eq):
>+        (JSC::JIT::compileOpEqJumpSlow):
>+        (JSC::JIT::emitSlow_op_neq):
>+        * jit/JITPropertyAccess.cpp:
>+        (JSC::JIT::stringGetByValStubGenerator):
>+        (JSC::JIT::emitSlow_op_get_by_val):
>+        (JSC::JIT::emitByValIdentifierCheck):
>+        * jit/JITPropertyAccess32_64.cpp:
>+        (JSC::JIT::stringGetByValStubGenerator):
>+        (JSC::JIT::emitSlow_op_get_by_val):
>+        * jit/JSInterfaceJIT.h:
>+        (JSC::ThunkHelpers::jsStringLengthOffset): Deleted.
>+        (JSC::ThunkHelpers::jsStringValueOffset): Deleted.
>+        * jit/SpecializedThunkJIT.h:
>+        (JSC::SpecializedThunkJIT::loadJSStringArgument):
>+        * jit/ThunkGenerators.cpp:
>+        (JSC::stringCharLoad):
>+        (JSC::charCodeAtThunkGenerator):
>+        (JSC::charAtThunkGenerator):
>+        * runtime/JSString.h:
>+
> 2018-05-20  Filip Pizlo  <fpizlo@apple.com>
> 
>         Revert the B3 compiler pipeline's treatment of taildup
>diff --git a/Source/JavaScriptCore/jit/JIT.h b/Source/JavaScriptCore/jit/JIT.h
>index b96aa22433aabda50ea8da9f3a995d904b797957..eca6d397766cf106b47b8bff8012c3398cf60808 100644
>--- a/Source/JavaScriptCore/jit/JIT.h
>+++ b/Source/JavaScriptCore/jit/JIT.h
>@@ -799,8 +799,6 @@ namespace JSC {
> 
>         void emitRightShiftFastPath(Instruction* currentInstruction, OpcodeID);
> 
>-        Jump checkStructure(RegisterID reg, Structure* structure);
>-
>         void updateTopCallFrame();
> 
>         Call emitNakedCall(CodePtr<NoPtrTag> function = CodePtr<NoPtrTag>());
>diff --git a/Source/JavaScriptCore/jit/JITInlines.h b/Source/JavaScriptCore/jit/JITInlines.h
>index 9a4b737c84aade6da37c371d8567eec9ef506055..475ed85a172a7ee395dc8c7a93342a0337e515d1 100644
>--- a/Source/JavaScriptCore/jit/JITInlines.h
>+++ b/Source/JavaScriptCore/jit/JITInlines.h
>@@ -99,9 +99,9 @@ ALWAYS_INLINE void JIT::emitPutIntToCallFrameHeader(RegisterID from, int entry)
> 
> ALWAYS_INLINE void JIT::emitLoadCharacterString(RegisterID src, RegisterID dst, JumpList& failures)
> {
>-    failures.append(branchStructure(NotEqual, Address(src, JSCell::structureIDOffset()), m_vm->stringStructure.get()));
>-    failures.append(branch32(NotEqual, MacroAssembler::Address(src, ThunkHelpers::jsStringLengthOffset()), TrustedImm32(1)));
>-    loadPtr(MacroAssembler::Address(src, ThunkHelpers::jsStringValueOffset()), dst);
>+    failures.append(branchIfNotString(src));
>+    failures.append(branch32(NotEqual, MacroAssembler::Address(src, JSString::offsetOfLength()), TrustedImm32(1)));
>+    loadPtr(MacroAssembler::Address(src, JSString::offsetOfValue()), dst);
>     failures.append(branchTest32(Zero, dst));
>     loadPtr(MacroAssembler::Address(dst, StringImpl::flagsOffset()), regT1);
>     loadPtr(MacroAssembler::Address(dst, StringImpl::dataOffset()), dst);
>@@ -198,11 +198,6 @@ ALWAYS_INLINE MacroAssembler::Call JIT::appendCallWithExceptionCheckSetJSValueRe
>     return call;
> }
> 
>-ALWAYS_INLINE JIT::Jump JIT::checkStructure(RegisterID reg, Structure* structure)
>-{
>-    return branchStructure(NotEqual, Address(reg, JSCell::structureIDOffset()), structure);
>-}
>-
> ALWAYS_INLINE void JIT::linkSlowCaseIfNotJSCell(Vector<SlowCaseEntry>::iterator& iter, int vReg)
> {
>     if (!m_codeBlock->isKnownNotImmediate(vReg))
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>index 44955e359b749978ede12eb8e80f4903eaa9d133..84f3a8154fa8743bf56ae26ce5b868fd2cf3fd38 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>@@ -461,8 +461,8 @@ void JIT::emitSlow_op_eq(Instruction* currentInstruction, Vector<SlowCaseEntry>:
>     genericCase.append(getSlowCase(iter)); // tags not equal
> 
>     linkSlowCase(iter); // tags equal and JSCell
>-    genericCase.append(branchPtr(NotEqual, Address(regT0, JSCell::structureIDOffset()), TrustedImmPtr(m_vm->stringStructure.get())));
>-    genericCase.append(branchPtr(NotEqual, Address(regT2, JSCell::structureIDOffset()), TrustedImmPtr(m_vm->stringStructure.get())));
>+    genericCase.append(branchIfNotString(regT0));
>+    genericCase.append(branchIfNotString(regT2));
> 
>     // String case.
>     callOperation(operationCompareStringEq, regT0, regT2);
>@@ -499,8 +499,8 @@ void JIT::compileOpEqJumpSlow(Vector<SlowCaseEntry>::iterator& iter, CompileOpEq
>     genericCase.append(getSlowCase(iter)); // tags not equal
> 
>     linkSlowCase(iter); // tags equal and JSCell
>-    genericCase.append(branchPtr(NotEqual, Address(regT0, JSCell::structureIDOffset()), TrustedImmPtr(m_vm->stringStructure.get())));
>-    genericCase.append(branchPtr(NotEqual, Address(regT2, JSCell::structureIDOffset()), TrustedImmPtr(m_vm->stringStructure.get())));
>+    genericCase.append(branchIfNotString(regT0));
>+    genericCase.append(branchIfNotString(regT2));
> 
>     // String case.
>     callOperation(operationCompareStringEq, regT0, regT2);
>@@ -547,8 +547,8 @@ void JIT::emitSlow_op_neq(Instruction* currentInstruction, Vector<SlowCaseEntry>
>     genericCase.append(getSlowCase(iter)); // tags not equal
> 
>     linkSlowCase(iter); // tags equal and JSCell
>-    genericCase.append(branchPtr(NotEqual, Address(regT0, JSCell::structureIDOffset()), TrustedImmPtr(m_vm->stringStructure.get())));
>-    genericCase.append(branchPtr(NotEqual, Address(regT2, JSCell::structureIDOffset()), TrustedImmPtr(m_vm->stringStructure.get())));
>+    genericCase.append(branchIfNotString(regT0));
>+    genericCase.append(branchIfNotString(regT2));
> 
>     // String case.
>     callOperation(operationCompareStringEq, regT0, regT2);
>diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>index e636f712090ff5098ed6143ea9b8c9a2f8097739..3efa388c653cb7fba668d4bfbb4ec41e8baa2a2c 100644
>--- a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>+++ b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>@@ -55,14 +55,11 @@ JIT::CodeRef<JITThunkPtrTag> JIT::stringGetByValStubGenerator(VM* vm)
>     JSInterfaceJIT jit(vm);
>     JumpList failures;
>     jit.tagReturnAddress();
>-    failures.append(jit.branchStructure(
>-        NotEqual, 
>-        Address(regT0, JSCell::structureIDOffset()), 
>-        vm->stringStructure.get()));
>+    failures.append(jit.branchIfNotString(regT0));
> 
>     // Load string length to regT2, and start the process of loading the data pointer into regT0
>-    jit.load32(Address(regT0, ThunkHelpers::jsStringLengthOffset()), regT2);
>-    jit.loadPtr(Address(regT0, ThunkHelpers::jsStringValueOffset()), regT0);
>+    jit.load32(Address(regT0, JSString::offsetOfLength()), regT2);
>+    jit.loadPtr(Address(regT0, JSString::offsetOfValue()), regT0);
>     failures.append(jit.branchTest32(Zero, regT0));
> 
>     // Do an unsigned compare to simultaneously filter negative indices as well as indices that are too large
>@@ -252,9 +249,7 @@ void JIT::emitSlow_op_get_by_val(Instruction* currentInstruction, Vector<SlowCas
>         linkSlowCase(iter); // property int32 check
>     Jump nonCell = jump();
>     linkSlowCase(iter); // base array check
>-    Jump notString = branchStructure(NotEqual, 
>-        Address(regT0, JSCell::structureIDOffset()), 
>-        m_vm->stringStructure.get());
>+    Jump notString = branchIfNotString(regT0);
>     emitNakedCall(CodeLocationLabel<NoPtrTag>(m_vm->getCTIStub(stringGetByValStubGenerator).retaggedCode<NoPtrTag>()));
>     Jump failed = branchTest64(Zero, regT0);
>     emitPutVirtualRegister(dst, regT0);
>@@ -1270,7 +1265,7 @@ void JIT::emitByValIdentifierCheck(ByValInfo* byValInfo, RegisterID cell, Regist
>     if (propertyName.isSymbol())
>         slowCases.append(branchPtr(NotEqual, cell, TrustedImmPtr(byValInfo->cachedSymbol.get())));
>     else {
>-        slowCases.append(branchStructure(NotEqual, Address(cell, JSCell::structureIDOffset()), m_vm->stringStructure.get()));
>+        slowCases.append(branchIfNotString(cell));
>         loadPtr(Address(cell, JSString::offsetOfValue()), scratch);
>         slowCases.append(branchPtr(NotEqual, scratch, TrustedImmPtr(propertyName.impl())));
>     }
>diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
>index 6041bc4e2d76c3c55e307042e95e14f80cb7b205..d45e57daa00dd186163b4e59880848aa1a957d07 100644
>--- a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
>+++ b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
>@@ -132,11 +132,11 @@ JIT::CodeRef<JITThunkPtrTag> JIT::stringGetByValStubGenerator(VM* vm)
> {
>     JSInterfaceJIT jit(vm);
>     JumpList failures;
>-    failures.append(jit.branchStructure(NotEqual, Address(regT0, JSCell::structureIDOffset()), vm->stringStructure.get()));
>+    failures.append(jit.branchIfNotString(regT0));
>     
>     // Load string length to regT1, and start the process of loading the data pointer into regT0
>-    jit.load32(Address(regT0, ThunkHelpers::jsStringLengthOffset()), regT1);
>-    jit.loadPtr(Address(regT0, ThunkHelpers::jsStringValueOffset()), regT0);
>+    jit.load32(Address(regT0, JSString::offsetOfLength()), regT1);
>+    jit.loadPtr(Address(regT0, JSString::offsetOfValue()), regT0);
>     failures.append(jit.branchTest32(Zero, regT0));
>     
>     // Do an unsigned compare to simultaneously filter negative indices as well as indices that are too large
>@@ -308,7 +308,7 @@ void JIT::emitSlow_op_get_by_val(Instruction* currentInstruction, Vector<SlowCas
> 
>     Jump nonCell = jump();
>     linkSlowCase(iter); // base array check
>-    Jump notString = branchStructure(NotEqual, Address(regT0, JSCell::structureIDOffset()), m_vm->stringStructure.get());
>+    Jump notString = branchIfNotString(regT0);
>     emitNakedCall(CodeLocationLabel<NoPtrTag>(m_vm->getCTIStub(stringGetByValStubGenerator).retaggedCode<NoPtrTag>()));
>     Jump failed = branchTestPtr(Zero, regT0);
>     emitStore(dst, regT1, regT0);
>diff --git a/Source/JavaScriptCore/jit/JSInterfaceJIT.h b/Source/JavaScriptCore/jit/JSInterfaceJIT.h
>index 2351973d4f9264eedf8d54467b3676df519ecd88..44746bbdb3fefad2bb5559a6ac3e0e75f0154b08 100644
>--- a/Source/JavaScriptCore/jit/JSInterfaceJIT.h
>+++ b/Source/JavaScriptCore/jit/JSInterfaceJIT.h
>@@ -77,11 +77,6 @@ namespace JSC {
>         VM* m_vm;
>     };
> 
>-    struct ThunkHelpers {
>-        static unsigned jsStringLengthOffset() { return OBJECT_OFFSETOF(JSString, m_length); }
>-        static unsigned jsStringValueOffset() { return OBJECT_OFFSETOF(JSString, m_value); }
>-    };
>-
> #if USE(JSVALUE32_64)
>     inline JSInterfaceJIT::Jump JSInterfaceJIT::emitLoadJSCell(unsigned virtualRegisterIndex, RegisterID payload)
>     {
>diff --git a/Source/JavaScriptCore/jit/SpecializedThunkJIT.h b/Source/JavaScriptCore/jit/SpecializedThunkJIT.h
>index f282321a469b05eec19e97d98ab71c473ff7f15d..4148d9e10f8092de9ee3a592b4e09d167b734a0f 100644
>--- a/Source/JavaScriptCore/jit/SpecializedThunkJIT.h
>+++ b/Source/JavaScriptCore/jit/SpecializedThunkJIT.h
>@@ -65,12 +65,10 @@ namespace JSC {
>             m_failures.append(emitLoadJSCell(src, dst));
>         }
>         
>-        void loadJSStringArgument(VM& vm, int argument, RegisterID dst)
>+        void loadJSStringArgument(int argument, RegisterID dst)
>         {
>             loadCellArgument(argument, dst);
>-            m_failures.append(branchStructure(NotEqual, 
>-                Address(dst, JSCell::structureIDOffset()), 
>-                vm.stringStructure.get()));
>+            m_failures.append(branchIfNotString(dst));
>         }
>         
>         void loadArgumentWithSpecificClass(const ClassInfo* classInfo, int argument, RegisterID dst, RegisterID scratch)
>diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.cpp b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>index 6e7e6313dcf3e0ebc0e87efd6270f2365ed90231..69fd2401f4e5328e847419597d941164694ac65e 100644
>--- a/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>+++ b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>@@ -617,14 +617,14 @@ MacroAssemblerCodeRef<JITThunkPtrTag> unreachableGenerator(VM* vm)
>     return FINALIZE_CODE(patchBuffer, JITThunkPtrTag, "unreachable thunk");
> }
> 
>-static void stringCharLoad(SpecializedThunkJIT& jit, VM* vm)
>+static void stringCharLoad(SpecializedThunkJIT& jit)
> {
>     // load string
>-    jit.loadJSStringArgument(*vm, SpecializedThunkJIT::ThisArgument, SpecializedThunkJIT::regT0);
>+    jit.loadJSStringArgument(SpecializedThunkJIT::ThisArgument, SpecializedThunkJIT::regT0);
> 
>     // Load string length to regT2, and start the process of loading the data pointer into regT0
>-    jit.load32(MacroAssembler::Address(SpecializedThunkJIT::regT0, ThunkHelpers::jsStringLengthOffset()), SpecializedThunkJIT::regT2);
>-    jit.loadPtr(MacroAssembler::Address(SpecializedThunkJIT::regT0, ThunkHelpers::jsStringValueOffset()), SpecializedThunkJIT::regT0);
>+    jit.load32(MacroAssembler::Address(SpecializedThunkJIT::regT0, JSString::offsetOfLength()), SpecializedThunkJIT::regT2);
>+    jit.loadPtr(MacroAssembler::Address(SpecializedThunkJIT::regT0, JSString::offsetOfValue()), SpecializedThunkJIT::regT0);
>     jit.appendFailure(jit.branchTest32(MacroAssembler::Zero, SpecializedThunkJIT::regT0));
> 
>     // load index
>@@ -658,7 +658,7 @@ static void charToString(SpecializedThunkJIT& jit, VM* vm, MacroAssembler::Regis
> MacroAssemblerCodeRef<JITThunkPtrTag> charCodeAtThunkGenerator(VM* vm)
> {
>     SpecializedThunkJIT jit(vm, 1);
>-    stringCharLoad(jit, vm);
>+    stringCharLoad(jit);
>     jit.returnInt32(SpecializedThunkJIT::regT0);
>     return jit.finalize(vm->jitStubs->ctiNativeTailCall(vm), "charCodeAt");
> }
>@@ -666,7 +666,7 @@ MacroAssemblerCodeRef<JITThunkPtrTag> charCodeAtThunkGenerator(VM* vm)
> MacroAssemblerCodeRef<JITThunkPtrTag> charAtThunkGenerator(VM* vm)
> {
>     SpecializedThunkJIT jit(vm, 1);
>-    stringCharLoad(jit, vm);
>+    stringCharLoad(jit);
>     charToString(jit, vm, SpecializedThunkJIT::regT0, SpecializedThunkJIT::regT0, SpecializedThunkJIT::regT1);
>     jit.returnJSCell(SpecializedThunkJIT::regT0);
>     return jit.finalize(vm->jitStubs->ctiNativeTailCall(vm), "charAt");
>diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
>index 4f65f636fa360fa91265fd2333e7941624737749..c755911806a3958607454d648dc707f52dbd149e 100644
>--- a/Source/JavaScriptCore/runtime/JSString.h
>+++ b/Source/JavaScriptCore/runtime/JSString.h
>@@ -80,7 +80,6 @@ class JSString : public JSCell {
>     friend class JSRopeString;
>     friend class MarkStack;
>     friend class SlotVisitor;
>-    friend struct ThunkHelpers;
> 
>     typedef JSCell Base;
>     static const unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | StructureIsImmortal | OverridesToThis;

Flags:

saam: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185810: 340821 | 340959