WebKit Bugzilla
Attachment 361529 Details for
Bug 194446
: Fix DFG's doesGC() for CheckTierUp*, GetByVal, PutByVal*, and StringCharAt nodes.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
proposed patch.
bug-194446.patch (text/plain), 12.28 KB, created by
Mark Lam
on 2019-02-08 13:19:21 PST
(
hide
)
Description:
proposed patch.
Filename:
MIME Type:
Creator:
Mark Lam
Created:
2019-02-08 13:19:21 PST
Size:
12.28 KB
patch
obsolete
>Index: Source/JavaScriptCore/ChangeLog >=================================================================== >--- Source/JavaScriptCore/ChangeLog (revision 241204) >+++ Source/JavaScriptCore/ChangeLog (working copy) >@@ -1,3 +1,56 @@ >+2019-02-08 Mark Lam <mark.lam@apple.com> >+ >+ Fix DFG's doesGC() for CheckTierUp*, GetByVal, PutByVal*, and StringCharAt nodes. >+ https://bugs.webkit.org/show_bug.cgi?id=194446 >+ <rdar://problem/47926792> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Fix doesGC() for the following nodes: >+ >+ CheckTierUpAtReturn: >+ Calls triggerTierUpNow(), which calls triggerFTLReplacementCompile(), >+ which calls Worklist::completeAllReadyPlansForVM(), which uses DeferGC. >+ >+ CheckTierUpInLoop: >+ Calls triggerTierUpNowInLoop(), which calls tierUpCommon(), which calls >+ Worklist::completeAllReadyPlansForVM(), which uses DeferGC. >+ >+ CheckTierUpAndOSREnter: >+ Calls triggerOSREntryNow(), which calls tierUpCommon(), which calls >+ Worklist::completeAllReadyPlansForVM(), which uses DeferGC. >+ >+ GetByVal: >+ case Array::String calls operationSingleCharacterString(), which calls >+ jsSingleCharacterString(), which can allocate a string. >+ >+ PutByValDirect: >+ PutByVal: >+ PutByValAlias: >+ For the DFG only, the integer TypeArrays calls compilePutByValForIntTypedArray(), >+ which may call slow paths operationPutByValDirectStrict(), operationPutByValDirectNonStrict(), >+ operationPutByValStrict(), or operationPutByValNonStrict(). All of these >+ slow paths call putByValInternal(), which may create exception objects, or >+ call the generic JSValue::put() which may execute arbitrary code. >+ >+ StringCharAt: >+ Can call operationSingleCharacterString(), which calls jsSingleCharacterString(), >+ which can allocate a string. >+ >+ Also fix DFG::SpeculativeJIT::compileGetByValOnString() and FTL's compileStringCharAt() >+ to use the maxSingleCharacterString constant instead of a literal constant. >+ >+ * dfg/DFGDoesGC.cpp: >+ (JSC::DFG::doesGC): >+ * dfg/DFGSpeculativeJIT.cpp: >+ (JSC::DFG::SpeculativeJIT::compileGetByValOnString): >+ * dfg/DFGSpeculativeJIT64.cpp: >+ (JSC::DFG::SpeculativeJIT::compile): >+ * ftl/FTLLowerDFGToB3.cpp: >+ (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal): >+ (JSC::FTL::DFG::LowerDFGToB3::compilePutByVal): >+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt): >+ > 2019-02-08 Yusuke Suzuki <ysuzuki@apple.com> > > [JSC] SourceProviderCacheItem should be small >Index: Source/JavaScriptCore/dfg/DFGDoesGC.cpp >=================================================================== >--- Source/JavaScriptCore/dfg/DFGDoesGC.cpp (revision 241204) >+++ Source/JavaScriptCore/dfg/DFGDoesGC.cpp (working copy) >@@ -53,6 +53,7 @@ bool doesGC(Graph& graph, Node* node) > // unless it is a known transition between previously allocated structures > // such as between Array types. > // 5. Calls to a JS function, which can execute arbitrary code including allocating objects. >+ // 6. Calls operations that uses DeferGC, because it may GC in its destructor. > > switch (node->op()) { > case JSConstant: >@@ -177,9 +178,6 @@ bool doesGC(Graph& graph, Node* node) > case ExtractOSREntryLocal: > case ExtractCatchLocal: > case ClearCatchLocals: >- case CheckTierUpInLoop: >- case CheckTierUpAtReturn: >- case CheckTierUpAndOSREnter: > case LoopHint: > case StoreBarrier: > case FencedStoreBarrier: >@@ -196,16 +194,11 @@ bool doesGC(Graph& graph, Node* node) > case Int52Rep: > case GetGetter: > case GetSetter: >- case GetByVal: > case GetArrayLength: > case GetVectorLength: >- case StringCharAt: > case StringCharCodeAt: > case GetTypedArrayByteOffset: > case GetPrototypeOf: >- case PutByValDirect: >- case PutByVal: >- case PutByValAlias: > case PutStructure: > case GetByOffset: > case GetGetterSetterByOffset: >@@ -272,6 +265,9 @@ bool doesGC(Graph& graph, Node* node) > case CallForwardVarargs: > case CallObjectConstructor: > case CallVarargs: >+ case CheckTierUpAndOSREnter: >+ case CheckTierUpAtReturn: >+ case CheckTierUpInLoop: > case Construct: > case ConstructForwardVarargs: > case ConstructVarargs: >@@ -325,6 +321,7 @@ bool doesGC(Graph& graph, Node* node) > case ResolveScope: > case ResolveScopeForHoistingFuncDeclInEval: > case Return: >+ case StringCharAt: > case TailCall: > case TailCallForwardVarargs: > case TailCallForwardVarargsInlinedCaller: >@@ -412,10 +409,30 @@ bool doesGC(Graph& graph, Node* node) > return true; > > case GetIndexedPropertyStorage: >+ case GetByVal: > if (node->arrayMode().type() == Array::String) > return true; > return false; > >+ case PutByValDirect: >+ case PutByVal: >+ case PutByValAlias: >+ if (!graph.m_plan.isFTL()) { >+ switch (node->arrayMode().modeForPut().type()) { >+ case Array::Int8Array: >+ case Array::Int16Array: >+ case Array::Int32Array: >+ case Array::Uint8Array: >+ case Array::Uint8ClampedArray: >+ case Array::Uint16Array: >+ case Array::Uint32Array: >+ return true; >+ default: >+ break; >+ } >+ } >+ return false; >+ > case MapHash: > switch (node->child1().useKind()) { > case BooleanUse: >Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp >=================================================================== >--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp (revision 241204) >+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp (working copy) >@@ -2344,8 +2344,11 @@ void SpeculativeJIT::compile(Node* node) > > case GetByVal: { > switch (node->arrayMode().type()) { >- case Array::SelectUsingPredictions: >+ case Array::AnyTypedArray: > case Array::ForceExit: >+ case Array::SelectUsingArguments: >+ case Array::SelectUsingPredictions: >+ case Array::Unprofiled: > DFG_CRASH(m_jit.graph(), node, "Bad array mode type"); > break; > case Array::Undecided: { >@@ -2566,7 +2569,15 @@ void SpeculativeJIT::compile(Node* node) > case Array::ScopedArguments: > compileGetByValOnScopedArguments(node); > break; >- default: { >+ case Array::Int8Array: >+ case Array::Int16Array: >+ case Array::Int32Array: >+ case Array::Uint8Array: >+ case Array::Uint8ClampedArray: >+ case Array::Uint16Array: >+ case Array::Uint32Array: >+ case Array::Float32Array: >+ case Array::Float64Array: { > TypedArrayType type = node->arrayMode().typedArrayType(); > if (isInt(type)) > compileGetByValOnIntTypedArray(node, type); >@@ -2800,14 +2811,35 @@ void SpeculativeJIT::compile(Node* node) > break; > } > >- default: { >+ case Array::Int8Array: >+ case Array::Int16Array: >+ case Array::Int32Array: >+ case Array::Uint8Array: >+ case Array::Uint8ClampedArray: >+ case Array::Uint16Array: >+ case Array::Uint32Array: >+ case Array::Float32Array: >+ case Array::Float64Array: { > TypedArrayType type = arrayMode.typedArrayType(); > if (isInt(type)) > compilePutByValForIntTypedArray(base.gpr(), property.gpr(), node, type); > else > compilePutByValForFloatTypedArray(base.gpr(), property.gpr(), node, type); >- } } >+ break; >+ } > >+ case Array::AnyTypedArray: >+ case Array::String: >+ case Array::DirectArguments: >+ case Array::ForceExit: >+ case Array::Generic: >+ case Array::ScopedArguments: >+ case Array::SelectUsingArguments: >+ case Array::SelectUsingPredictions: >+ case Array::Undecided: >+ case Array::Unprofiled: >+ RELEASE_ASSERT_NOT_REACHED(); >+ } > break; > } > >Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp >=================================================================== >--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (revision 241204) >+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (working copy) >@@ -2194,7 +2194,7 @@ void SpeculativeJIT::compileGetByValOnSt > m_jit.load16(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesTwo, 0), scratchReg); > > JITCompiler::Jump bigCharacter = >- m_jit.branch32(MacroAssembler::AboveOrEqual, scratchReg, TrustedImm32(0x100)); >+ m_jit.branch32(MacroAssembler::Above, scratchReg, TrustedImm32(maxSingleCharacterString)); > > // 8 bit string values don't need the isASCII check. > cont8Bit.link(&m_jit); >Index: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >=================================================================== >--- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (revision 241204) >+++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (working copy) >@@ -4163,13 +4163,21 @@ private: > return; > } > >- default: { >+ case Array::Int8Array: >+ case Array::Int16Array: >+ case Array::Int32Array: >+ case Array::Uint8Array: >+ case Array::Uint8ClampedArray: >+ case Array::Uint16Array: >+ case Array::Uint32Array: >+ case Array::Float32Array: >+ case Array::Float64Array: { > LValue index = lowInt32(m_graph.varArgChild(m_node, 1)); > LValue storage = lowStorage(m_graph.varArgChild(m_node, 2)); > > TypedArrayType type = m_node->arrayMode().typedArrayType(); >- >- if (isTypedView(type)) { >+ ASSERT(isTypedView(type)); >+ { > TypedPointer pointer = pointerIntoTypedArray(storage, index, type); > > if (isInt(type)) { >@@ -4196,10 +4204,16 @@ private: > setDouble(result); > return; > } >- >+ } >+ >+ case Array::AnyTypedArray: >+ case Array::ForceExit: >+ case Array::SelectUsingArguments: >+ case Array::SelectUsingPredictions: >+ case Array::Unprofiled: > DFG_CRASH(m_graph, m_node, "Bad array type"); > return; >- } } >+ } > } > > void compileGetMyArgumentByVal() >@@ -4488,10 +4502,19 @@ private: > return; > } > >- default: { >+ case Array::Int8Array: >+ case Array::Int16Array: >+ case Array::Int32Array: >+ case Array::Uint8Array: >+ case Array::Uint8ClampedArray: >+ case Array::Uint16Array: >+ case Array::Uint32Array: >+ case Array::Float32Array: >+ case Array::Float64Array: { > TypedArrayType type = arrayMode.typedArrayType(); > >- if (isTypedView(type)) { >+ ASSERT(isTypedView(type)); >+ { > TypedPointer pointer = TypedPointer( > m_heaps.typedArrayProperties, > m_out.add( >@@ -4544,11 +4567,21 @@ private: > > return; > } >+ } > >+ case Array::AnyTypedArray: >+ case Array::String: >+ case Array::DirectArguments: >+ case Array::ForceExit: >+ case Array::Generic: >+ case Array::ScopedArguments: >+ case Array::SelectUsingArguments: >+ case Array::SelectUsingPredictions: >+ case Array::Undecided: >+ case Array::Unprofiled: > DFG_CRASH(m_graph, m_node, "Bad array type"); > break; > } >- } > } > > void compilePutAccessorById() >@@ -6579,7 +6612,7 @@ private: > provenValue(m_graph.child(m_node, 1)))); > ValueFromBlock char16Bit = m_out.anchor(char16BitValue); > m_out.branch( >- m_out.aboveOrEqual(char16BitValue, m_out.constInt32(0x100)), >+ m_out.above(char16BitValue, m_out.constInt32(maxSingleCharacterString)), > rarely(bigCharacter), usually(bitsContinuation)); > > m_out.appendTo(bigCharacter, bitsContinuation);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
saam
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194446
: 361529