WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
proposed patch
blah.patch (text/plain), 22.73 KB, created by
Filip Pizlo
on 2014-06-11 17:44:10 PDT
(
hide
)
Description:
proposed patch
Filename:
MIME Type:
Creator:
Filip Pizlo
Created:
2014-06-11 17:44:10 PDT
Size:
22.73 KB
patch
obsolete
>Index: Source/JavaScriptCore/ChangeLog >=================================================================== >--- Source/JavaScriptCore/ChangeLog (revision 169859) >+++ Source/JavaScriptCore/ChangeLog (working copy) >@@ -1,3 +1,36 @@ >+2014-06-11 Filip Pizlo <fpizlo@apple.com> >+ >+ [ftlopt] DFG get_by_id should inline chain accesses with a slightly polymorphic base >+ https://bugs.webkit.org/show_bug.cgi?id=133751 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * bytecode/GetByIdStatus.cpp: >+ (JSC::GetByIdStatus::appendVariant): >+ (JSC::GetByIdStatus::computeForStubInfo): >+ * bytecode/GetByIdVariant.cpp: >+ (JSC::GetByIdVariant::attemptToMerge): >+ * bytecode/GetByIdVariant.h: >+ * bytecode/PutByIdStatus.cpp: >+ (JSC::PutByIdStatus::computeFor): >+ * dfg/DFGByteCodeParser.cpp: >+ (JSC::DFG::ByteCodeParser::emitPrototypeChecks): >+ (JSC::DFG::ByteCodeParser::handleGetById): >+ (JSC::DFG::ByteCodeParser::handlePutById): >+ * runtime/IntendedStructureChain.cpp: >+ (JSC::IntendedStructureChain::IntendedStructureChain): >+ (JSC::IntendedStructureChain::isStillValid): >+ (JSC::IntendedStructureChain::isNormalized): >+ (JSC::IntendedStructureChain::terminalPrototype): >+ (JSC::IntendedStructureChain::operator==): >+ (JSC::IntendedStructureChain::visitChildren): >+ (JSC::IntendedStructureChain::dumpInContext): >+ (JSC::IntendedStructureChain::chain): Deleted. >+ * runtime/IntendedStructureChain.h: >+ (JSC::IntendedStructureChain::prototype): >+ (JSC::IntendedStructureChain::operator!=): >+ (JSC::IntendedStructureChain::head): Deleted. >+ > 2014-06-11 Matthew Mirman <mmirman@apple.com> > > Ensured Native Calls and Construct and associated checks >Index: Source/JavaScriptCore/bytecode/GetByIdStatus.cpp >=================================================================== >--- Source/JavaScriptCore/bytecode/GetByIdStatus.cpp (revision 169812) >+++ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp (working copy) >@@ -39,10 +39,20 @@ namespace JSC { > > bool GetByIdStatus::appendVariant(const GetByIdVariant& variant) > { >+ // Attempt to merge this variant with an already existing variant. >+ for (unsigned i = 0; i < m_variants.size(); ++i) { >+ if (m_variants[i].attemptToMerge(variant)) >+ return true; >+ } >+ >+ // Make sure there is no overlap. We should have pruned out opportunities for >+ // overlap but it's possible that an inline cache got into a weird state. We are >+ // defensive and bail if we detect crazy. > for (unsigned i = 0; i < m_variants.size(); ++i) { > if (m_variants[i].structureSet().overlaps(variant.structureSet())) > return false; > } >+ > m_variants.append(variant); > return true; > } >@@ -187,10 +197,12 @@ GetByIdStatus GetByIdStatus::computeForS > profiledBlock, structure, list->at(listIndex).chain(), > list->at(listIndex).chainCount())); > >- if (!chain->isStillValid()) >- return GetByIdStatus(slowPathState, true); >+ if (!chain->isStillValid()) { >+ // This won't ever run again so skip it. >+ continue; >+ } > >- if (chain->head()->takesSlowPathInDFGForImpureProperty()) >+ if (structure->takesSlowPathInDFGForImpureProperty()) > return GetByIdStatus(slowPathState, true); > > size_t chainSize = chain->size(); >@@ -217,35 +229,6 @@ GetByIdStatus GetByIdStatus::computeForS > > if (!isValidOffset(myOffset)) > return GetByIdStatus(slowPathState, true); >- >- if (!chain && !list->at(listIndex).doesCalls()) { >- // For non-chain, non-getter accesses, we try to do some coalescing. >- bool found = false; >- for (unsigned variantIndex = 0; variantIndex < result.m_variants.size(); ++variantIndex) { >- GetByIdVariant& variant = result.m_variants[variantIndex]; >- if (variant.m_chain) >- continue; >- >- if (variant.m_offset != myOffset) >- continue; >- >- if (variant.callLinkStatus()) >- continue; >- >- found = true; >- if (variant.m_structureSet.contains(structure)) >- break; >- >- if (variant.m_specificValue != JSValue(specificValue)) >- variant.m_specificValue = JSValue(); >- >- variant.m_structureSet.add(structure); >- break; >- } >- >- if (found) >- continue; >- } > > std::unique_ptr<CallLinkStatus> callLinkStatus; > switch (list->at(listIndex).type()) { >@@ -272,6 +255,7 @@ GetByIdStatus GetByIdStatus::computeForS > GetByIdVariant variant( > StructureSet(structure), myOffset, specificValue, chain, > std::move(callLinkStatus)); >+ > if (!result.appendVariant(variant)) > return GetByIdStatus(slowPathState, true); > } >Index: Source/JavaScriptCore/bytecode/GetByIdVariant.cpp >=================================================================== >--- Source/JavaScriptCore/bytecode/GetByIdVariant.cpp (revision 169812) >+++ Source/JavaScriptCore/bytecode/GetByIdVariant.cpp (working copy) >@@ -51,6 +51,25 @@ GetByIdVariant& GetByIdVariant::operator > return *this; > } > >+bool GetByIdVariant::attemptToMerge(const GetByIdVariant& other) >+{ >+ if (!!m_chain != !!other.m_chain) >+ return false; >+ if (m_chain && *m_chain != *other.m_chain) >+ return false; >+ if (m_offset != other.m_offset) >+ return false; >+ if (m_callLinkStatus || other.m_callLinkStatus) >+ return false; >+ >+ if (m_specificValue != other.m_specificValue) >+ m_specificValue = JSValue(); >+ >+ m_structureSet.merge(other.m_structureSet); >+ >+ return true; >+} >+ > void GetByIdVariant::dump(PrintStream& out) const > { > dumpInContext(out, 0); >Index: Source/JavaScriptCore/bytecode/GetByIdVariant.h >=================================================================== >--- Source/JavaScriptCore/bytecode/GetByIdVariant.h (revision 169812) >+++ Source/JavaScriptCore/bytecode/GetByIdVariant.h (working copy) >@@ -71,6 +71,8 @@ public: > PropertyOffset offset() const { return m_offset; } > CallLinkStatus* callLinkStatus() const { return m_callLinkStatus.get(); } > >+ bool attemptToMerge(const GetByIdVariant& other); >+ > void dump(PrintStream&) const; > void dumpInContext(PrintStream&, DumpContext*) const; > >Index: Source/JavaScriptCore/bytecode/PutByIdStatus.cpp >=================================================================== >--- Source/JavaScriptCore/bytecode/PutByIdStatus.cpp (revision 169812) >+++ Source/JavaScriptCore/bytecode/PutByIdStatus.cpp (working copy) >@@ -309,7 +309,7 @@ PutByIdStatus PutByIdStatus::computeFor( > // dictionaries if we have evidence to suggest that those objects were never used as > // prototypes in a cacheable prototype access - i.e. there's a good chance that some of > // the other checks below will fail. >- if (!chain->isNormalized()) >+ if (structure->isProxy() || !chain->isNormalized()) > return PutByIdStatus(TakesSlowPath); > } > >Index: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >=================================================================== >--- Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (revision 169812) >+++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (working copy) >@@ -192,7 +192,7 @@ private: > void handlePutById( > Node* base, unsigned identifierNumber, Node* value, const PutByIdStatus&, > bool isDirect); >- Node* emitPrototypeChecks(Structure*, IntendedStructureChain*); >+ Node* emitPrototypeChecks(IntendedStructureChain*); > > Node* getScope(bool skipTop, unsigned skipCount); > >@@ -1694,18 +1694,18 @@ Node* ByteCodeParser::handlePutByOffset( > return result; > } > >-Node* ByteCodeParser::emitPrototypeChecks( >- Structure* structure, IntendedStructureChain* chain) >+Node* ByteCodeParser::emitPrototypeChecks(IntendedStructureChain* chain) > { >- ASSERT(structure); >- Node* base = 0; > m_graph.chains().addLazily(chain); >- Structure* currentStructure = structure; >- JSObject* currentObject = 0; >+ JSValue prototype = chain->prototype(); >+ if (prototype.isNull()) >+ return nullptr; >+ Node* base = nullptr; > for (unsigned i = 0; i < chain->size(); ++i) { >- currentObject = asObject(currentStructure->prototypeForLookup(m_inlineStackTop->m_codeBlock)); >- currentStructure = chain->at(i); >+ JSObject* currentObject = asObject(prototype); >+ Structure* currentStructure = chain->at(i); > base = cellConstantWithStructureCheck(currentObject, currentStructure); >+ prototype = currentStructure->prototypeForLookup(m_inlineStackTop->m_codeBlock); > } > return base; > } >@@ -1737,11 +1737,8 @@ void ByteCodeParser::handleGetById( > // optimal, if there is some rarely executed case in the chain that requires a lot > // of checks and those checks are not watchpointable. > for (unsigned variantIndex = getByIdStatus.numVariants(); variantIndex--;) { >- if (getByIdStatus[variantIndex].chain()) { >- emitPrototypeChecks( >- getByIdStatus[variantIndex].structureSet().onlyStructure(), >- getByIdStatus[variantIndex].chain()); >- } >+ if (getByIdStatus[variantIndex].chain()) >+ emitPrototypeChecks(getByIdStatus[variantIndex].chain()); > } > > // 2) Emit a MultiGetByOffset >@@ -1763,10 +1760,8 @@ void ByteCodeParser::handleGetById( > > addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(variant.structureSet())), base); > >- if (variant.chain()) { >- base = emitPrototypeChecks( >- variant.structureSet().onlyStructure(), variant.chain()); >- } >+ if (variant.chain()) >+ base = emitPrototypeChecks(variant.chain()); > > // Unless we want bugs like https://bugs.webkit.org/show_bug.cgi?id=88783, we need to > // ensure that the base of the original get_by_id is kept alive until we're done with >@@ -1865,9 +1860,7 @@ void ByteCodeParser::handlePutById( > continue; > if (!putByIdStatus[variantIndex].structureChain()) > continue; >- emitPrototypeChecks( >- putByIdStatus[variantIndex].oldStructure(), >- putByIdStatus[variantIndex].structureChain()); >+ emitPrototypeChecks(putByIdStatus[variantIndex].structureChain()); > } > } > >@@ -1903,7 +1896,7 @@ void ByteCodeParser::handlePutById( > > addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(variant.oldStructure())), base); > if (!isDirect) >- emitPrototypeChecks(variant.oldStructure(), variant.structureChain()); >+ emitPrototypeChecks(variant.structureChain()); > > ASSERT(variant.oldStructure()->transitionWatchpointSetHasBeenInvalidated()); > >Index: Source/JavaScriptCore/runtime/IntendedStructureChain.cpp >=================================================================== >--- Source/JavaScriptCore/runtime/IntendedStructureChain.cpp (revision 169812) >+++ Source/JavaScriptCore/runtime/IntendedStructureChain.cpp (working copy) >@@ -33,27 +33,37 @@ > > namespace JSC { > >-IntendedStructureChain::IntendedStructureChain(JSGlobalObject* globalObject, Structure* head) >+IntendedStructureChain::IntendedStructureChain(JSGlobalObject* globalObject, JSValue prototype) > : m_globalObject(globalObject) >- , m_head(head) >+ , m_prototype(prototype) > { >- JSValue prototype = head->prototypeForLookup(globalObject); >+ ASSERT(m_prototype.isNull() || m_prototype.isObject()); > if (prototype.isNull()) > return; > for (Structure* current = asObject(prototype)->structure(); current; current = current->storedPrototypeStructure()) > m_vector.append(current); > } > >+IntendedStructureChain::IntendedStructureChain(JSGlobalObject* globalObject, Structure* head) >+ : m_globalObject(globalObject) >+ , m_prototype(head->prototypeForLookup(m_globalObject)) >+{ >+ if (m_prototype.isNull()) >+ return; >+ for (Structure* current = asObject(m_prototype)->structure(); current; current = current->storedPrototypeStructure()) >+ m_vector.append(current); >+} >+ > IntendedStructureChain::IntendedStructureChain(CodeBlock* codeBlock, Structure* head, Structure* prototypeStructure) > : m_globalObject(codeBlock->globalObject()) >- , m_head(head) >+ , m_prototype(head->prototypeForLookup(m_globalObject)) > { > m_vector.append(prototypeStructure); > } > > IntendedStructureChain::IntendedStructureChain(CodeBlock* codeBlock, Structure* head, StructureChain* chain) > : m_globalObject(codeBlock->globalObject()) >- , m_head(head) >+ , m_prototype(head->prototypeForLookup(m_globalObject)) > { > for (unsigned i = 0; chain->head()[i]; ++i) > m_vector.append(chain->head()[i].get()); >@@ -61,7 +71,7 @@ IntendedStructureChain::IntendedStructur > > IntendedStructureChain::IntendedStructureChain(CodeBlock* codeBlock, Structure* head, StructureChain* chain, unsigned count) > : m_globalObject(codeBlock->globalObject()) >- , m_head(head) >+ , m_prototype(head->prototypeForLookup(m_globalObject)) > { > for (unsigned i = 0; i < count; ++i) > m_vector.append(chain->head()[i].get()); >@@ -73,7 +83,7 @@ IntendedStructureChain::~IntendedStructu > > bool IntendedStructureChain::isStillValid() const > { >- JSValue currentPrototype = m_head->prototypeForLookup(m_globalObject); >+ JSValue currentPrototype = m_prototype; > for (unsigned i = 0; i < m_vector.size(); ++i) { > if (asObject(currentPrototype)->structure() != m_vector[i]) > return false; >@@ -93,14 +103,6 @@ bool IntendedStructureChain::matches(Str > return true; > } > >-StructureChain* IntendedStructureChain::chain(VM& vm) const >-{ >- ASSERT(isStillValid()); >- StructureChain* result = StructureChain::create(vm, m_head); >- ASSERT(matches(result)); >- return result; >-} >- > bool IntendedStructureChain::mayInterceptStoreTo(VM& vm, StringImpl* uid) > { > for (unsigned i = 0; i < m_vector.size(); ++i) { >@@ -118,8 +120,6 @@ bool IntendedStructureChain::mayIntercep > > bool IntendedStructureChain::isNormalized() > { >- if (m_head->isProxy()) >- return false; > for (unsigned i = 0; i < m_vector.size(); ++i) { > Structure* structure = m_vector[i]; > if (structure->isProxy()) >@@ -134,14 +134,21 @@ JSObject* IntendedStructureChain::termin > { > ASSERT(!m_vector.isEmpty()); > if (m_vector.size() == 1) >- return asObject(m_head->prototypeForLookup(m_globalObject)); >+ return asObject(m_prototype); > return asObject(m_vector[m_vector.size() - 2]->storedPrototype()); > } > >+bool IntendedStructureChain::operator==(const IntendedStructureChain& other) const >+{ >+ return m_globalObject == other.m_globalObject >+ && m_prototype == other.m_prototype >+ && m_vector == other.m_vector; >+} >+ > void IntendedStructureChain::visitChildren(SlotVisitor& visitor) > { > visitor.appendUnbarrieredPointer(&m_globalObject); >- visitor.appendUnbarrieredPointer(&m_head); >+ visitor.appendUnbarrieredValue(&m_prototype); > for (unsigned i = m_vector.size(); i--;) > visitor.appendUnbarrieredPointer(&m_vector[i]); > } >@@ -155,7 +162,7 @@ void IntendedStructureChain::dumpInConte > { > out.print( > "(global = ", RawPointer(m_globalObject), ", head = ", >- pointerDumpInContext(m_head, context), ", vector = ["); >+ inContext(m_prototype, context), ", vector = ["); > CommaPrinter comma; > for (unsigned i = 0; i < m_vector.size(); ++i) > out.print(comma, pointerDumpInContext(m_vector[i], context)); >Index: Source/JavaScriptCore/runtime/IntendedStructureChain.h >=================================================================== >--- Source/JavaScriptCore/runtime/IntendedStructureChain.h (revision 169812) >+++ Source/JavaScriptCore/runtime/IntendedStructureChain.h (working copy) >@@ -39,7 +39,8 @@ struct DumpContext; > > class IntendedStructureChain : public RefCounted<IntendedStructureChain> { > public: >- IntendedStructureChain(JSGlobalObject* globalObject, Structure* head); >+ IntendedStructureChain(JSGlobalObject* globalObject, JSValue prototype); >+ IntendedStructureChain(JSGlobalObject* globalObject, Structure*); > IntendedStructureChain(CodeBlock* codeBlock, Structure* head, Structure* prototypeStructure); > IntendedStructureChain(CodeBlock* codeBlock, Structure* head, StructureChain* chain); > IntendedStructureChain(CodeBlock* codeBlock, Structure* head, StructureChain* chain, unsigned count); >@@ -47,11 +48,10 @@ public: > > bool isStillValid() const; > bool matches(StructureChain*) const; >- StructureChain* chain(VM&) const; > bool mayInterceptStoreTo(VM&, StringImpl* uid); > bool isNormalized(); > >- Structure* head() const { return m_head; } >+ JSValue prototype() const { return m_prototype; } > > size_t size() const { return m_vector.size(); } > Structure* at(size_t index) { return m_vector[index]; } >@@ -61,13 +61,19 @@ public: > > Structure* last() const { return m_vector.last(); } > >+ bool operator==(const IntendedStructureChain&) const; >+ bool operator!=(const IntendedStructureChain& other) const >+ { >+ return !(*this == other); >+ } >+ > void visitChildren(SlotVisitor&); > void dump(PrintStream&) const; > void dumpInContext(PrintStream&, DumpContext*) const; > > private: > JSGlobalObject* m_globalObject; >- Structure* m_head; >+ JSValue m_prototype; > Vector<Structure*> m_vector; > }; > >Index: LayoutTests/ChangeLog >=================================================================== >--- LayoutTests/ChangeLog (revision 169859) >+++ LayoutTests/ChangeLog (working copy) >@@ -1,3 +1,17 @@ >+2014-06-11 Filip Pizlo <fpizlo@apple.com> >+ >+ [ftlopt] DFG get_by_id should inline chain accesses with a slightly polymorphic base >+ https://bugs.webkit.org/show_bug.cgi?id=133751 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * js/regress/poly-chain-access-expected.txt: Added. >+ * js/regress/poly-chain-access-simpler-expected.txt: Added. >+ * js/regress/poly-chain-access-simpler.html: Added. >+ * js/regress/poly-chain-access.html: Added. >+ * js/regress/script-tests/poly-chain-access-simpler.js: Added. >+ * js/regress/script-tests/poly-chain-access.js: Added. >+ > 2014-06-01 Filip Pizlo <fpizlo@apple.com> > > [ftlopt] AI should be able track structure sets larger than 1 >Index: LayoutTests/js/regress/poly-chain-access-expected.txt >=================================================================== >--- LayoutTests/js/regress/poly-chain-access-expected.txt (revision 0) >+++ LayoutTests/js/regress/poly-chain-access-expected.txt (working copy) >@@ -0,0 +1,10 @@ >+JSRegress/poly-chain-access >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+PASS no exception thrown >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >Index: LayoutTests/js/regress/poly-chain-access-simpler-expected.txt >=================================================================== >--- LayoutTests/js/regress/poly-chain-access-simpler-expected.txt (revision 0) >+++ LayoutTests/js/regress/poly-chain-access-simpler-expected.txt (working copy) >@@ -0,0 +1,10 @@ >+JSRegress/poly-chain-access-simpler >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+PASS no exception thrown >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >Index: LayoutTests/js/regress/poly-chain-access-simpler.html >=================================================================== >--- LayoutTests/js/regress/poly-chain-access-simpler.html (revision 0) >+++ LayoutTests/js/regress/poly-chain-access-simpler.html (working copy) >@@ -0,0 +1,12 @@ >+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> >+<html> >+<head> >+<script src="../../resources/js-test-pre.js"></script> >+</head> >+<body> >+<script src="../../resources/regress-pre.js"></script> >+<script src="script-tests/poly-chain-access-simpler.js"></script> >+<script src="../../resources/regress-post.js"></script> >+<script src="../../resources/js-test-post.js"></script> >+</body> >+</html> >Index: LayoutTests/js/regress/poly-chain-access.html >=================================================================== >--- LayoutTests/js/regress/poly-chain-access.html (revision 0) >+++ LayoutTests/js/regress/poly-chain-access.html (working copy) >@@ -0,0 +1,12 @@ >+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> >+<html> >+<head> >+<script src="../../resources/js-test-pre.js"></script> >+</head> >+<body> >+<script src="../../resources/regress-pre.js"></script> >+<script src="script-tests/poly-chain-access.js"></script> >+<script src="../../resources/regress-post.js"></script> >+<script src="../../resources/js-test-post.js"></script> >+</body> >+</html> >Index: LayoutTests/js/regress/script-tests/poly-chain-access-simpler.js >=================================================================== >--- LayoutTests/js/regress/script-tests/poly-chain-access-simpler.js (revision 0) >+++ LayoutTests/js/regress/script-tests/poly-chain-access-simpler.js (working copy) >@@ -0,0 +1,26 @@ >+(function() { >+ function Foo() { } >+ Foo.prototype.f = 42; >+ function Bar() { } >+ Bar.prototype = new Foo(); >+ >+ function foo(o, p) { >+ var n = 1000000; >+ var result = 0; >+ for (var i = 0; i < n; ++i) { >+ result += o.f; >+ var tmp = o; >+ o = p; >+ p = tmp; >+ } >+ >+ if (result != n * 42) >+ throw "Error: bad result: " + result; >+ } >+ >+ var o = new Bar(); >+ var p = new Bar(); >+ p.g = 43; >+ >+ foo(o, p); >+})(); >Index: LayoutTests/js/regress/script-tests/poly-chain-access.js >=================================================================== >--- LayoutTests/js/regress/script-tests/poly-chain-access.js (revision 0) >+++ LayoutTests/js/regress/script-tests/poly-chain-access.js (working copy) >@@ -0,0 +1,22 @@ >+(function() { >+ function Foo() { } >+ Foo.prototype.f = 42; >+ function Bar() { } >+ Bar.prototype = new Foo(); >+ >+ var o = new Bar(); >+ var p = new Bar(); >+ p.g = 43; >+ >+ var n = 1000000; >+ var result = 0; >+ for (var i = 0; i < n; ++i) { >+ result += o.f; >+ var tmp = o; >+ o = p; >+ p = tmp; >+ } >+ >+ if (result != n * 42) >+ throw "Error: bad result: " + result; >+})();
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=232924">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Flags:
mhahnenberg
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 133751
: 232924