Attachment 341038 Details for Bug 185864 – Patch

[patch] Patch

bug-185864-20180523080009.patch (text/plain), 12.23 KB, created by Yusuke Suzuki on 2018-05-22 16:00:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-22 16:00:10 PDT

Size: 12.23 KB

patch

obsolete

>Subversion Revision: 232081
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4fc72fe9c1f6fa29bdbded6c1dff55545b940ee8..0ca8dba3b5da8cfc85c13f4e17bb515c84b5c6c8 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-05-22  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [JSC] Clean up stringGetByValStubGenerator
>+        https://bugs.webkit.org/show_bug.cgi?id=185864
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We clean up stringGetByValStubGenerator.
>+
>+        1. Unify 32bit and 64bit implementations.
>+        2. Rename stringGetByValStubGenerator to stringGetByValGenerator, move it to ThunkGenerators.cpp.
>+        3. Remove string type check since this code is invoked only when we know regT0 is JSString*.
>+        4. Do not tag Cell in stringGetByValGenerator side. 32bit code stores Cell with tag in JITPropertyAccess32_64 side.
>+        5. Fix invalid use of loadPtr for StringImpl::flags. Should use load32.
>+
>+        * jit/JIT.h:
>+        * jit/JITPropertyAccess.cpp:
>+        (JSC::JIT::emitSlow_op_get_by_val):
>+        (JSC::JIT::stringGetByValStubGenerator): Deleted.
>+        * jit/JITPropertyAccess32_64.cpp:
>+        (JSC::JIT::emit_op_get_by_val):
>+        (JSC::JIT::emitSlow_op_get_by_val):
>+        (JSC::JIT::stringGetByValStubGenerator): Deleted.
>+        * jit/ThunkGenerators.cpp:
>+        (JSC::stringGetByValGenerator):
>+        * jit/ThunkGenerators.h:
>+
> 2018-05-22  Keith Miller  <keith_miller@apple.com>
> 
>         Remove the UnconditionalFinalizer class
>diff --git a/Source/JavaScriptCore/jit/JIT.h b/Source/JavaScriptCore/jit/JIT.h
>index c3852222c5aa1248bd601273a6bd21bc4cf951b0..fee12e7805d6c5d162ee373f00184d8b409992de 100644
>--- a/Source/JavaScriptCore/jit/JIT.h
>+++ b/Source/JavaScriptCore/jit/JIT.h
>@@ -892,7 +892,6 @@ namespace JSC {
> 
>         std::unique_ptr<JITDisassembler> m_disassembler;
>         RefPtr<Profiler::Compilation> m_compilation;
>-        static CodeRef<JITThunkPtrTag> stringGetByValStubGenerator(VM*);
> 
>         PCToCodeOriginMapBuilder m_pcToCodeOriginMapBuilder;
> 
>diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>index ec792f4adf655a30b6ae60d8dfc7ee375565f54e..f973603cdc01f208085a415e39c157b8e2e58767 100644
>--- a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>+++ b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>@@ -50,50 +50,6 @@
> namespace JSC {
> #if USE(JSVALUE64)
> 
>-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()));
>-
>-    // 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);
>-    failures.append(jit.branchTest32(Zero, regT0));
>-
>-    // Do an unsigned compare to simultaneously filter negative indices as well as indices that are too large
>-    failures.append(jit.branch32(AboveOrEqual, regT1, regT2));
>-    
>-    // Load the character
>-    JumpList is16Bit;
>-    JumpList cont8Bit;
>-    // Load the string flags
>-    jit.loadPtr(Address(regT0, StringImpl::flagsOffset()), regT2);
>-    jit.loadPtr(Address(regT0, StringImpl::dataOffset()), regT0);
>-    is16Bit.append(jit.branchTest32(Zero, regT2, TrustedImm32(StringImpl::flagIs8Bit())));
>-    jit.load8(BaseIndex(regT0, regT1, TimesOne, 0), regT0);
>-    cont8Bit.append(jit.jump());
>-    is16Bit.link(&jit);
>-    jit.load16(BaseIndex(regT0, regT1, TimesTwo, 0), regT0);
>-    cont8Bit.link(&jit);
>-
>-    failures.append(jit.branch32(AboveOrEqual, regT0, TrustedImm32(0x100)));
>-    jit.move(TrustedImmPtr(vm->smallStrings.singleCharacterStrings()), regT1);
>-    jit.loadPtr(BaseIndex(regT1, regT0, ScalePtr, 0), regT0);
>-    jit.ret();
>-    
>-    failures.link(&jit);
>-    jit.move(TrustedImm32(0), regT0);
>-    jit.ret();
>-    
>-    LinkBuffer patchBuffer(jit, GLOBAL_THUNK_ID);
>-    return FINALIZE_CODE(patchBuffer, JITThunkPtrTag, "String get_by_val stub");
>-}
>-
> void JIT::emit_op_get_by_val(Instruction* currentInstruction)
> {
>     int dst = currentInstruction[1].u.operand;
>@@ -255,7 +211,7 @@ void JIT::emitSlow_op_get_by_val(Instruction* currentInstruction, Vector<SlowCas
>     Jump notString = branchStructure(NotEqual, 
>         Address(regT0, JSCell::structureIDOffset()), 
>         m_vm->stringStructure.get());
>-    emitNakedCall(CodeLocationLabel<NoPtrTag>(m_vm->getCTIStub(stringGetByValStubGenerator).retaggedCode<NoPtrTag>()));
>+    emitNakedCall(CodeLocationLabel<NoPtrTag>(m_vm->getCTIStub(stringGetByValGenerator).retaggedCode<NoPtrTag>()));
>     Jump failed = branchTest64(Zero, regT0);
>     emitPutVirtualRegister(dst, regT0);
>     emitJumpSlowToHot(jump(), OPCODE_LENGTH(op_get_by_val));
>diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
>index a70a1551b23c04e985ee0dfc0d93837e9e011b86..5d19a85413a3f0ae1d93cb519f3afb90ec7efa02 100644
>--- a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
>+++ b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
>@@ -128,48 +128,6 @@ void JIT::emit_op_del_by_val(Instruction* currentInstruction)
>     callOperation(operationDeleteByValJSResult, dst, JSValueRegs(regT1, regT0), JSValueRegs(regT3, regT2));
> }
> 
>-JIT::CodeRef<JITThunkPtrTag> JIT::stringGetByValStubGenerator(VM* vm)
>-{
>-    JSInterfaceJIT jit(vm);
>-    JumpList failures;
>-    failures.append(jit.branchStructure(NotEqual, Address(regT0, JSCell::structureIDOffset()), vm->stringStructure.get()));
>-    
>-    // 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);
>-    failures.append(jit.branchTest32(Zero, regT0));
>-    
>-    // Do an unsigned compare to simultaneously filter negative indices as well as indices that are too large
>-    failures.append(jit.branch32(AboveOrEqual, regT2, regT1));
>-    
>-    // Load the character
>-    JumpList is16Bit;
>-    JumpList cont8Bit;
>-    // Load the string flags
>-    jit.loadPtr(Address(regT0, StringImpl::flagsOffset()), regT1);
>-    jit.loadPtr(Address(regT0, StringImpl::dataOffset()), regT0);
>-    is16Bit.append(jit.branchTest32(Zero, regT1, TrustedImm32(StringImpl::flagIs8Bit())));
>-    jit.load8(BaseIndex(regT0, regT2, TimesOne, 0), regT0);
>-    cont8Bit.append(jit.jump());
>-    is16Bit.link(&jit);
>-    jit.load16(BaseIndex(regT0, regT2, TimesTwo, 0), regT0);
>-
>-    cont8Bit.link(&jit);
>-    
>-    failures.append(jit.branch32(AboveOrEqual, regT0, TrustedImm32(0x100)));
>-    jit.move(TrustedImmPtr(vm->smallStrings.singleCharacterStrings()), regT1);
>-    jit.loadPtr(BaseIndex(regT1, regT0, ScalePtr, 0), regT0);
>-    jit.move(TrustedImm32(JSValue::CellTag), regT1); // We null check regT0 on return so this is safe
>-    jit.ret();
>-
>-    failures.link(&jit);
>-    jit.move(TrustedImm32(0), regT0);
>-    jit.ret();
>-    
>-    LinkBuffer patchBuffer(jit, GLOBAL_THUNK_ID);
>-    return FINALIZE_CODE(patchBuffer, JITThunkPtrTag, "String get_by_val stub");
>-}
>-
> void JIT::emit_op_get_by_val(Instruction* currentInstruction)
> {
>     int dst = currentInstruction[1].u.operand;
>@@ -177,7 +135,7 @@ void JIT::emit_op_get_by_val(Instruction* currentInstruction)
>     int property = currentInstruction[3].u.operand;
>     ArrayProfile* profile = currentInstruction[4].u.arrayProfile;
>     ByValInfo* byValInfo = m_codeBlock->addByValInfo();
>-    
>+
>     emitLoad2(base, regT1, regT0, property, regT3, regT2);
>     
>     emitJumpSlowCaseIfNotJSCell(base, regT1);
>@@ -309,9 +267,9 @@ 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());
>-    emitNakedCall(CodeLocationLabel<NoPtrTag>(m_vm->getCTIStub(stringGetByValStubGenerator).retaggedCode<NoPtrTag>()));
>+    emitNakedCall(CodeLocationLabel<NoPtrTag>(m_vm->getCTIStub(stringGetByValGenerator).retaggedCode<NoPtrTag>()));
>     Jump failed = branchTestPtr(Zero, regT0);
>-    emitStore(dst, regT1, regT0);
>+    emitStoreCell(dst, regT0);
>     emitJumpSlowToHot(jump(), OPCODE_LENGTH(op_get_by_val));
>     failed.link(this);
>     notString.link(this);
>diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.cpp b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>index f59a4fff7dc402a7498fc16dfdd4c835a66b73cd..1edae2871736f8fdca0f7b03a9fed87f827f4e7a 100644
>--- a/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>+++ b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>@@ -612,6 +612,58 @@ MacroAssemblerCodeRef<JITThunkPtrTag> unreachableGenerator(VM* vm)
>     return FINALIZE_CODE(patchBuffer, JITThunkPtrTag, "unreachable thunk");
> }
> 
>+MacroAssemblerCodeRef<JITThunkPtrTag> stringGetByValGenerator(VM* vm)
>+{
>+    // regT0 is JSString*, and regT1 (64bit) or regT2 (32bit) is int index.
>+    // Return regT0 = result JSString* if succeeds. Otherwise, return regT0 = 0.
>+#if USE(JSVALUE64)
>+    GPRReg stringGPR = regT0;
>+    GPRReg indexGPR = regT1;
>+    GPRReg scratchGPR = regT2;
>+#else
>+    GPRReg stringGPR = regT0;
>+    GPRReg indexGPR = regT2;
>+    GPRReg scratchGPR = regT1;
>+#endif
>+
>+    JSInterfaceJIT jit(vm);
>+    JSInterfaceJIT::JumpList failures;
>+    jit.tagReturnAddress();
>+
>+    // Load string length to regT2, and start the process of loading the data pointer into regT0
>+    jit.load32(JSInterfaceJIT::Address(stringGPR, JSString::offsetOfLength()), scratchGPR);
>+    jit.loadPtr(JSInterfaceJIT::Address(stringGPR, JSString::offsetOfValue()), stringGPR);
>+    failures.append(jit.branchTestPtr(JSInterfaceJIT::Zero, stringGPR));
>+
>+    // Do an unsigned compare to simultaneously filter negative indices as well as indices that are too large
>+    failures.append(jit.branch32(JSInterfaceJIT::AboveOrEqual, indexGPR, scratchGPR));
>+
>+    // Load the character
>+    JSInterfaceJIT::JumpList is16Bit;
>+    JSInterfaceJIT::JumpList cont8Bit;
>+    // Load the string flags
>+    jit.load32(JSInterfaceJIT::Address(stringGPR, StringImpl::flagsOffset()), scratchGPR);
>+    jit.loadPtr(JSInterfaceJIT::Address(stringGPR, StringImpl::dataOffset()), stringGPR);
>+    is16Bit.append(jit.branchTest32(JSInterfaceJIT::Zero, scratchGPR, JSInterfaceJIT::TrustedImm32(StringImpl::flagIs8Bit())));
>+    jit.load8(JSInterfaceJIT::BaseIndex(stringGPR, indexGPR, JSInterfaceJIT::TimesOne, 0), stringGPR);
>+    cont8Bit.append(jit.jump());
>+    is16Bit.link(&jit);
>+    jit.load16(JSInterfaceJIT::BaseIndex(stringGPR, indexGPR, JSInterfaceJIT::TimesTwo, 0), stringGPR);
>+    cont8Bit.link(&jit);
>+
>+    failures.append(jit.branch32(JSInterfaceJIT::AboveOrEqual, stringGPR, JSInterfaceJIT::TrustedImm32(0x100)));
>+    jit.move(JSInterfaceJIT::TrustedImmPtr(vm->smallStrings.singleCharacterStrings()), indexGPR);
>+    jit.loadPtr(JSInterfaceJIT::BaseIndex(indexGPR, stringGPR, JSInterfaceJIT::ScalePtr, 0), stringGPR);
>+    jit.ret();
>+
>+    failures.link(&jit);
>+    jit.move(JSInterfaceJIT::TrustedImm32(0), stringGPR);
>+    jit.ret();
>+
>+    LinkBuffer patchBuffer(jit, GLOBAL_THUNK_ID);
>+    return FINALIZE_CODE(patchBuffer, JITThunkPtrTag, "String get_by_val stub");
>+}
>+
> static void stringCharLoad(SpecializedThunkJIT& jit, VM* vm)
> {
>     // load string
>diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.h b/Source/JavaScriptCore/jit/ThunkGenerators.h
>index eb32e737cb71c2c9ec628688aebd5ea01aeef90f..2d4b30f179fcd72ed49d97abb3d689cfb5cf2bf3 100644
>--- a/Source/JavaScriptCore/jit/ThunkGenerators.h
>+++ b/Source/JavaScriptCore/jit/ThunkGenerators.h
>@@ -51,6 +51,7 @@ MacroAssemblerCodeRef<JITThunkPtrTag> internalFunctionCallGenerator(VM*);
> MacroAssemblerCodeRef<JITThunkPtrTag> internalFunctionConstructGenerator(VM*);
> MacroAssemblerCodeRef<JITThunkPtrTag> arityFixupGenerator(VM*);
> MacroAssemblerCodeRef<JITThunkPtrTag> unreachableGenerator(VM*);
>+MacroAssemblerCodeRef<JITThunkPtrTag> stringGetByValGenerator(VM*);
> 
> MacroAssemblerCodeRef<JITThunkPtrTag> charCodeAtThunkGenerator(VM*);
> MacroAssemblerCodeRef<JITThunkPtrTag> charAtThunkGenerator(VM*);

Actions: View | Formatted Diff | Diff

Attachments on bug 185864: 340975 | 340988 | 340997 | 341038 | 341041