[DFG][FTL] Introduce StringSlice
Created attachment 325150 [details] Patch
Comment on attachment 325150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325150&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2919 > + if (argumentCountIncludingThis != 2) Nit: even though they’re equivalent in this context, I think > 2 is more intuitive > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4309 > + // end would be nullptr. would => can > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4320 > + LValue startIndex = m_out.select(m_out.greaterThanOrEqual(start, m_out.int32Zero), These two boundary selections are equivalent if parameterized on start/end. Might be worth making a lambda
Comment on attachment 325150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325150&action=review > Source/JavaScriptCore/ChangeLog:13 > + 1. Empty string generation is accelerated. It is fully executed inlinely. inlinely =>inline
Comment on attachment 325150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325150&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:13 >> + 1. Empty string generation is accelerated. It is fully executed inlinely. > > inlinely =>inline Fixed. >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2919 >> + if (argumentCountIncludingThis != 2) > > Nit: even though they’re equivalent in this context, I think > 2 is more intuitive Changed. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4309 >> + // end would be nullptr. > > would => can Fixed. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4320 >> + LValue startIndex = m_out.select(m_out.greaterThanOrEqual(start, m_out.int32Zero), > > These two boundary selections are equivalent if parameterized on start/end. Might be worth making a lambda Yeah, nice. Fixed.
Committed r224276: <https://trac.webkit.org/changeset/224276>
This change introduced an assertion failure with LayoutTest inspector/formatting/formatting-javascript.html: https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK1%20(Tests)/r224277%20(4602)/results.html ASSERTION FAILED: graph.m_plan.weakReferences.contains(cell) ./ftl/FTLOutput.h(112) : LValue JSC::FTL::Output::weakPointer(DFG::Graph &, JSC::JSCell *) 1 0x10f31c9f0 WTFCrash 2 0x10e9abd12 JSC::FTL::Output::weakPointer(JSC::DFG::Graph&, JSC::JSCell*) 3 0x10e9288e9 JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileStringSlice() 4 0x10e8ece68 JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileNode(unsigned int) 5 0x10e8e7bf0 JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileBlock(JSC::DFG::BasicBlock*) 6 0x10e8e3a56 JSC::FTL::(anonymous namespace)::LowerDFGToB3::lower() 7 0x10e8e274e JSC::FTL::lowerDFGToB3(JSC::FTL::State&) 8 0x10e728b37 JSC::DFG::Plan::compileInThreadImpl() 9 0x10e72367d JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) 10 0x10e8b755e JSC::DFG::Worklist::ThreadBody::work() 11 0x10f32a3fe WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const 12 0x10f32a0dc WTF::Function<void ()>::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0>::call() 13 0x10f35a8fe WTF::Function<void ()>::operator()() const 14 0x10f3a96ea WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) 15 0x10f3af0d5 WTF::wtfThreadEntryPoint(void*) 16 0x7fff8536b99d _pthread_body 17 0x7fff8536b91a _pthread_body 18 0x7fff85369351 thread_start The same assertion is also seen with 121 tests on the Debug JSC bot: https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20JSC%20%28Tests%29/builds/1479
Committed r224285: <https://trac.webkit.org/changeset/224285>
(In reply to Yusuke Suzuki from comment #7) > Committed r224285: <https://trac.webkit.org/changeset/224285> Maybe it’s worth making FTLOutput’s weakPointer private and just friend lower’s weakPointer? Or maybe we can use naming to help prevent this mistake in the future in a case where it actually matters.
(In reply to Saam Barati from comment #8) > (In reply to Yusuke Suzuki from comment #7) > > Committed r224285: <https://trac.webkit.org/changeset/224285> > > Maybe it’s worth making FTLOutput’s weakPointer private and just friend > lower’s weakPointer? Or maybe we can use naming to help prevent this mistake > in the future in a case where it actually matters. It sound nice. I'll open a bug for that.
<rdar://problem/35567861>