Attachment 340975 Details for Bug 185864 – Patch

[patch] Patch

bug-185864-20180522212153.patch (text/plain), 11.57 KB, created by Yusuke Suzuki on 2018-05-22 05:21:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-22 05:21:54 PDT

Size: 11.57 KB

patch

obsolete

>Subversion Revision: 232059
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4a8e76f81fb93d9a81585985eb4bd9b592c0a7ef..091be3479c1c30bc1b6d07a99fb14bf85578fb25 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+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.
>+
>+        * jit/JIT.h:
>+        * jit/JITPropertyAccess.cpp:
>+        (JSC::JIT::emitSlow_op_get_by_val):
>+        (JSC::JIT::stringGetByValStubGenerator): Deleted.
>+        * jit/JITPropertyAccess32_64.cpp:
>+        (JSC::JIT::emitSlow_op_get_by_val):
>+        (JSC::JIT::stringGetByValStubGenerator): Deleted.
>+        * jit/ThunkGenerators.cpp:
>+        (JSC::stringGetByValGenerator):
>+        * jit/ThunkGenerators.h:
>+
> 2018-05-21  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         Use more C++17
>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 e636f712090ff5098ed6143ea9b8c9a2f8097739..7418c0bb88089a0eb97ec27499764f4f98e3885c 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..ef4fb64ad4c0d4a7fd1fa14bcbbee408ba32e7ab 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;
>@@ -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..9796b39b1e1a649bc2adead900dd2834e494bc9a 100644
>--- a/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>+++ b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
>@@ -612,6 +612,48 @@ MacroAssemblerCodeRef<JITThunkPtrTag> unreachableGenerator(VM* vm)
>     return FINALIZE_CODE(patchBuffer, JITThunkPtrTag, "unreachable thunk");
> }
> 
>+MacroAssemblerCodeRef<JITThunkPtrTag> stringGetByValGenerator(VM* vm)
>+{
>+    // regT0 is always JSString*. Return regT0 = result JSString* if succeeds.
>+    // Otherwise, return regT0 = 0.
>+    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(GPRInfo::regT0, JSString::offsetOfLength()), GPRInfo::regT2);
>+    jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::regT0, JSString::offsetOfValue()), GPRInfo::regT0);
>+    failures.append(jit.branchTest32(JSInterfaceJIT::Zero, GPRInfo::regT0));
>+
>+    // Do an unsigned compare to simultaneously filter negative indices as well as indices that are too large
>+    failures.append(jit.branch32(JSInterfaceJIT::AboveOrEqual, GPRInfo::regT1, GPRInfo::regT2));
>+
>+    // Load the character
>+    JSInterfaceJIT::JumpList is16Bit;
>+    JSInterfaceJIT::JumpList cont8Bit;
>+    // Load the string flags
>+    jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::regT0, StringImpl::flagsOffset()), GPRInfo::regT2);
>+    jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::regT0, StringImpl::dataOffset()), GPRInfo::regT0);
>+    is16Bit.append(jit.branchTest32(JSInterfaceJIT::Zero, GPRInfo::regT2, JSInterfaceJIT::TrustedImm32(StringImpl::flagIs8Bit())));
>+    jit.load8(JSInterfaceJIT::BaseIndex(GPRInfo::regT0, GPRInfo::regT1, JSInterfaceJIT::TimesOne, 0), GPRInfo::regT0);
>+    cont8Bit.append(jit.jump());
>+    is16Bit.link(&jit);
>+    jit.load16(JSInterfaceJIT::BaseIndex(GPRInfo::regT0, GPRInfo::regT1, JSInterfaceJIT::TimesTwo, 0), GPRInfo::regT0);
>+    cont8Bit.link(&jit);
>+
>+    failures.append(jit.branch32(JSInterfaceJIT::AboveOrEqual, GPRInfo::regT0, JSInterfaceJIT::TrustedImm32(0x100)));
>+    jit.move(JSInterfaceJIT::TrustedImmPtr(vm->smallStrings.singleCharacterStrings()), GPRInfo::regT1);
>+    jit.loadPtr(JSInterfaceJIT::BaseIndex(GPRInfo::regT1, GPRInfo::regT0, JSInterfaceJIT::ScalePtr, 0), GPRInfo::regT0);
>+    jit.ret();
>+
>+    failures.link(&jit);
>+    jit.move(JSInterfaceJIT::TrustedImm32(0), GPRInfo::regT0);
>+    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