Let's implement DOMJIT Patchpoints for firstChild, lastChild, nextSibling, previousSibling, parentNode.
Created attachment 290894 [details] Patch WIP: initial working prototype
Attachment 290894 [details] did not pass style-queue: ERROR: Source/WebCore/ForwardingHeaders/domjit/DOMJITPatchpoint.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5] ERROR: Source/WebCore/ForwardingHeaders/domjit/DOMJITPatchpointParams.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5] Total errors found: 2 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 290894 [details] Patch Attachment 290894 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2235633 New failing tests: imported/w3c/web-platform-tests/html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
Created attachment 290902 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 290894 [details] Patch Attachment 290894 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2235646 New failing tests: imported/w3c/web-platform-tests/html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
Created attachment 290903 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 290907 [details] Patch
Attachment 290907 [details] did not pass style-queue: ERROR: Source/WebCore/ForwardingHeaders/domjit/DOMJITPatchpoint.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5] ERROR: Source/WebCore/ForwardingHeaders/domjit/DOMJITPatchpointParams.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5] Total errors found: 2 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 290908 [details] Patch
Comment on attachment 290908 [details] Patch Attachment 290908 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2236277 New failing tests: imported/w3c/web-platform-tests/html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
Created attachment 290914 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 290908 [details] Patch Attachment 290908 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2236285 New failing tests: imported/w3c/web-platform-tests/html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
Created attachment 290915 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 290942 [details] Patch
Created attachment 290945 [details] Patch
Comment on attachment 290945 [details] Patch Attachment 290945 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2239016 New failing tests: imported/w3c/web-platform-tests/html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
Created attachment 290954 [details] Archive of layout-test-results from ews106 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 290945 [details] Patch Attachment 290945 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2239060 New failing tests: imported/w3c/web-platform-tests/html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
Created attachment 290956 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 290960 [details] Patch
Created attachment 290985 [details] Patch
Cool!
Comment on attachment 290985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290985&action=review > Source/JavaScriptCore/domjit/DOMJITPatchpoint.h:65 > + bool useTagMaskRegister { true }; > + bool useTagTypeNumberRegister { true }; Can we just make these always true and remove them? > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:8737 > + if (domJIT->useTagMaskRegister) > + patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister)); > + if (domJIT->useTagTypeNumberRegister) > + patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister)); It's relatively cheap to do this, so we could just do it all of the time. I think that's what the get_by_id/put_by_id patchpoints do, for example. Heck I think we also do it for ValueAdd and friends when emitting them in snippet (i.e. patchpoint) mode. Also, why are you making them late registers? I think it's correct, but stronger than what we need. If the result is Void then you don't have to worry about them interfering with result. If the result is not Void and you are already using SomeEarlyRegister for the result, then the result is already going to interfere with early uses. Also, early uses already interfere with scratches. It's OK to keep the code this way, since it's telling a sound truth, but I'm curious if you needed it to solve some bug! > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:8784 > + if (domJIT->useTagMaskRegister) > + patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister)); > + if (domJIT->useTagTypeNumberRegister) > + patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister)); Ditto. > Source/WTF/wtf/StdLibExtras.h:71 > +#define CAST_OFFSET(from, to) (reinterpret_cast<uintptr_t>(static_cast<to>((reinterpret_cast<from>(0x4000)))) - 0x4000) Nice. That's so evil. > Source/WebCore/domjit/DOMJITHelpers.h:42 > +inline CCallHelpers::Jump branchIfNotWorldIsNormal(CCallHelpers& jit, GPRReg globalObject) I love this style. I wish that we had written CCallHelpers and AssemblyHelpers this way.
Comment on attachment 290985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290985&action=review >> Source/JavaScriptCore/domjit/DOMJITPatchpoint.h:65 >> + bool useTagTypeNumberRegister { true }; > > Can we just make these always true and remove them? OK. Dropped. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:8737 >> + patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister)); > > It's relatively cheap to do this, so we could just do it all of the time. I think that's what the get_by_id/put_by_id patchpoints do, for example. Heck I think we also do it for ValueAdd and friends when emitting them in snippet (i.e. patchpoint) mode. > > Also, why are you making them late registers? I think it's correct, but stronger than what we need. If the result is Void then you don't have to worry about them interfering with result. If the result is not Void and you are already using SomeEarlyRegister for the result, then the result is already going to interfere with early uses. Also, early uses already interfere with scratches. It's OK to keep the code this way, since it's telling a sound truth, but I'm curious if you needed it to solve some bug! Right. resultConstraint change is used to make the patchpoint environment in FTL closer to one in DFG: the result reg interferes with early uses. On the other hand, this change does not fix any bugs. The previous one just works. And you're right. This patchpoint already said that the result is SomeEarlyRegister. So we can just use ValueRep::reg() and they interfere with the other early uses and the result. Changed it to ValueRep::reg and always added this constraint. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:8784 >> + patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister)); > > Ditto. Fixed. >> Source/WebCore/domjit/DOMJITHelpers.h:42 >> +inline CCallHelpers::Jump branchIfNotWorldIsNormal(CCallHelpers& jit, GPRReg globalObject) > > I love this style. I wish that we had written CCallHelpers and AssemblyHelpers this way. Yeah, this allows us to extend helpers easily in everywhere (including outside of JSC like WebCore)
Created attachment 291136 [details] Patch for landing
Committed r207013: <http://trac.webkit.org/changeset/207013>