136936 2014-09-18 19:29:57 -0700 CSS JIT: Don't add backtracking register count to minimum register count if it's not necessary 2014-09-26 08:04:29 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 1 ysuzuki ysuzuki benjamin oldest_to_newest 1036050 0 ysuzuki 2014-09-18 19:29:57 -0700 minimumRequiredRegisterCount always contains `BacktrackingRegister`. However when the fragment is not inside InChainWithDescendantTail, this register is not allocated. So adding `backtrackingRegisterRequirements` only if it's necessary. 1036051 1 238348 ysuzuki 2014-09-18 19:34:43 -0700 Created attachment 238348 Patch 1036059 2 benjamin 2014-09-18 22:19:02 -0700 That would be useful for ARMv7. But we would need to re-check all the generators because everything was coded with the assumption of 6 for the rightmost fragment. For example, I believe :nth-child() uses 6 registers on x86 when doing style resolution for the rightmost selector. I can check that tomorrow. In any case, we should land this after https://bugs.webkit.org/show_bug.cgi?id=136933 to have better chance of catching mistakes. 1036060 3 ysuzuki 2014-09-18 22:22:35 -0700 (In reply to comment #2) > That would be useful for ARMv7. But we would need to re-check all the generators because everything was coded with the assumption of 6 for the rightmost fragment. > > For example, I believe :nth-child() uses 6 registers on x86 when doing style resolution for the rightmost selector. I can check that tomorrow. > > In any case, we should land this after https://bugs.webkit.org/show_bug.cgi?id=136933 to have better chance of catching mistakes. You're right. That patch[1] points out invalid assumption by raising assertion failures. It makes this patch safer. [1]: https://bugs.webkit.org/show_bug.cgi?id=136933 1037113 4 ysuzuki 2014-09-24 18:27:38 -0700 To ensure this patch pass the test under the https://bugs.webkit.org/show_bug.cgi?id=136933, I'll re-upload the same patch. 1037114 5 238633 ysuzuki 2014-09-24 18:28:03 -0700 Created attachment 238633 Patch 1037355 6 238633 benjamin 2014-09-25 17:29:49 -0700 Comment on attachment 238633 Patch Clearing the review flag. The patch is likely correct, but I don't think the gain/risk is worth it at this very moment. My rationale: All the code has been verified for 6 registers for the rightmost fragment and 5 for backtracking. Verifying all the generators for 5 registers would be very time consuming, it would probably take me an entire day of review. We are on the verge of adding complex new capabilities to the engine. There are at least two features that will need great attention: 1) matching multiple pseudo elements with :matches() 2) Multilevel backtracking for :nth-child(), :matches() and :not(). The minus side of not including this patch is some performance impact on ARMv7. But even today without it we are several times faster than any competing engine. 1037481 7 ysuzuki 2014-09-26 08:04:29 -0700 (In reply to comment #6) > (From update of attachment 238633 [details]) > Clearing the review flag. > > The patch is likely correct, but I don't think the gain/risk is worth it at this very moment. > > My rationale: > > All the code has been verified for 6 registers for the rightmost fragment and 5 for backtracking. Verifying all the generators for 5 registers would be very time consuming, it would probably take me an entire day of review. > > We are on the verge of adding complex new capabilities to the engine. There are at least two features that will need great attention: 1) matching multiple pseudo elements with :matches() 2) Multilevel backtracking for :nth-child(), :matches() and :not(). > > The minus side of not including this patch is some performance impact on ARMv7. But even today without it we are several times faster than any competing engine. Make sense. Yup, that's reasonable decision. So closing this issue :) 238348 2014-09-18 19:34:43 -0700 2014-09-24 18:28:00 -0700 Patch bug-136936-20140919113440.patch text/plain 3095 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMTczNzQ2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTNiMTMyOTMzMGU0ZWMz YWQ0ZTIxNjIwYzQ3Yzc5OWNkZTE3ZmUzMy4uMjg5ODM4NjNlNmU1MDRjZTU1Mjg5MTE2OGQ2ZDlm YThiY2I0YTI4NiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE0LTA5LTE4ICBZdXN1 a2UgU3V6dWtpICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgorCisgICAgICAgIENTUyBKSVQ6IERv bid0IGFkZCBiYWNrdHJhY2tpbmcgcmVnaXN0ZXIgY291bnQgdG8gbWluaW11bSByZWdpc3RlciBj b3VudCBpZiBpdCdzIG5vdCBuZWNlc3NhcnkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEzNjkzNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIE5vdyBgbWluaW11bVJlcXVpcmVkUmVnaXN0ZXJDb3VudGAgYWx3 YXlzIGNvbnRhaW5zIGBCYWNrdHJhY2tpbmdSZWdpc3RlcmAuCisgICAgICAgIEhvd2V2ZXIgd2hl biB0aGUgZnJhZ21lbnQgaXMgbm90IGluc2lkZSBJbkNoYWluV2l0aERlc2NlbmRhbnRUYWlsLAor ICAgICAgICB0aGlzIGJhY2t0cmFja2luZyByZWdpc3RlciBpcyBub3QgYWN0dWFsbHkgYWxsb2Nh dGVkLgorICAgICAgICBTbyBhZGRpbmcgYGJhY2t0cmFja2luZ1JlZ2lzdGVyUmVxdWlyZW1lbnRz YCBvbmx5IGlmIGl0J3MgbmVjZXNzYXJ5CisgICAgICAgIHRvIHJlZHVjZSB0aGUgdW5uZWNlc3Nh cnkgcmVnaXN0ZXIgcHJlc3N1cmUuCisKKyAgICAgICAgKiBjc3NqaXQvU2VsZWN0b3JDb21waWxl ci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTZWxlY3RvckNvbXBpbGVyOjptaW5pbXVtUmVnaXN0 ZXJSZXF1aXJlbWVudHMpOgorCiAyMDE0LTA5LTE4ICBSb2dlciBGb25nICA8cm9nZXJfZm9uZ0Bh cHBsZS5jb20+CiAKICAgICAgICAgW1dpbmRvd3NdIE1lZGlhIHRlc3RzIGFsbCBmYWlsIGJlY2F1 c2UgdGhleSBjYW5ub3QgZmluZCBsb2NhbGl6ZWQgc3RyaW5ncy4KZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJDb3JlL2Nzc2ppdC9TZWxlY3RvckNvbXBpbGVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nz c2ppdC9TZWxlY3RvckNvbXBpbGVyLmNwcAppbmRleCAxODVjMjU4NTMzYTM1ZDYxZDU1ZmY4Njkz ZThhMTU0ZmNhMGYwZjA2Li5lY2YxYWFkMGJlMTk0OTExMzc1Mzc4OTBhZDMyZWRmYWMwYzAyZThl IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9jc3NqaXQvU2VsZWN0b3JDb21waWxlci5jcHAK KysrIGIvU291cmNlL1dlYkNvcmUvY3Nzaml0L1NlbGVjdG9yQ29tcGlsZXIuY3BwCkBAIC04NDMs MjEgKzg0MywyMSBAQCBzdGF0aWMgaW5saW5lIGJvb2wgYXR0cmlidXRlVmFsdWVUZXN0aW5nUmVx dWlyZXNDYXNlRm9sZGluZ1JlZ2lzdGVyKGNvbnN0IEF0dHJpYgogfQogCiAvLyBTdHJpY3QgbWlu aW11bSB0byBtYXRjaCBhbnl0aGluZyBpbnRlcmVzdGluZzoKLS8vIEVsZW1lbnQgKyBCYWNrdHJh Y2tpbmdSZWdpc3RlciArIEVsZW1lbnREYXRhICsgYSBwb2ludGVyIHRvIHZhbHVlcyArIGFuIGlu ZGV4IG9uIHRoYXQgcG9pbnRlciArIHRoZSB2YWx1ZSB3ZSBleHBlY3Q7Ci1zdGF0aWMgY29uc3Qg dW5zaWduZWQgbWluaW11bVJlcXVpcmVkUmVnaXN0ZXJDb3VudCA9IDY7CisvLyBFbGVtZW50ICsg RWxlbWVudERhdGEgKyBhIHBvaW50ZXIgdG8gdmFsdWVzICsgYW4gaW5kZXggb24gdGhhdCBwb2lu dGVyICsgdGhlIHZhbHVlIHdlIGV4cGVjdDsKK3N0YXRpYyBjb25zdCB1bnNpZ25lZCBtaW5pbXVt UmVxdWlyZWRSZWdpc3RlckNvdW50ID0gNTsKIC8vIEVsZW1lbnQgKyBFbGVtZW50RGF0YSArIHNj cmF0Y2hSZWdpc3RlciArIGF0dHJpYnV0ZUFycmF5UG9pbnRlciArIGV4cGVjdGVkTG9jYWxOYW1l ICsgKHF1YWxpZmllZE5hbWVJbXBsICYmIGV4cGVjdGVkVmFsdWUpLgogc3RhdGljIGNvbnN0IHVu c2lnbmVkIG1pbmltdW1SZXF1aXJlZFJlZ2lzdGVyQ291bnRGb3JBdHRyaWJ1dGVGaWx0ZXIgPSA2 OwogCiBzdGF0aWMgaW5saW5lIHVuc2lnbmVkIG1pbmltdW1SZWdpc3RlclJlcXVpcmVtZW50cyhj b25zdCBTZWxlY3RvckZyYWdtZW50JiBzZWxlY3RvckZyYWdtZW50KQogewotICAgIHVuc2lnbmVk IG1pbmltdW0gPSBtaW5pbXVtUmVxdWlyZWRSZWdpc3RlckNvdW50OwotICAgIGNvbnN0IFZlY3Rv cjxBdHRyaWJ1dGVNYXRjaGluZ0luZm8+JiBhdHRyaWJ1dGVzID0gc2VsZWN0b3JGcmFnbWVudC5h dHRyaWJ1dGVzOwotCiAgICAgdW5zaWduZWQgYmFja3RyYWNraW5nUmVnaXN0ZXJSZXF1aXJlbWVu dHMgPSAwOwogICAgIGlmIChzZWxlY3RvckZyYWdtZW50LmJhY2t0cmFja2luZ0ZsYWdzICYgQmFj a3RyYWNraW5nRmxhZzo6SW5DaGFpbldpdGhEZXNjZW5kYW50VGFpbCkKICAgICAgICAgYmFja3Ry YWNraW5nUmVnaXN0ZXJSZXF1aXJlbWVudHMgPSAxOyAvLyBJZiB0aGVyZSBpcyBhIERlc2NlbmRh bnRUYWlsLCB0aGVyZSBpcyBhIGJhY2t0cmFja2luZyByZWdpc3Rlci4KIAorICAgIHVuc2lnbmVk IG1pbmltdW0gPSBtaW5pbXVtUmVxdWlyZWRSZWdpc3RlckNvdW50ICsgYmFja3RyYWNraW5nUmVn aXN0ZXJSZXF1aXJlbWVudHM7CisKICAgICAvLyBBdHRyaWJ1dGVzIGNhdXNlIHNvbWUgcmVnaXN0 ZXIgcHJlc3N1cmUuCisgICAgY29uc3QgVmVjdG9yPEF0dHJpYnV0ZU1hdGNoaW5nSW5mbz4mIGF0 dHJpYnV0ZXMgPSBzZWxlY3RvckZyYWdtZW50LmF0dHJpYnV0ZXM7CiAgICAgdW5zaWduZWQgYXR0 cmlidXRlQ291bnQgPSBhdHRyaWJ1dGVzLnNpemUoKTsKICAgICBmb3IgKHVuc2lnbmVkIGF0dHJp YnV0ZUluZGV4ID0gMDsgYXR0cmlidXRlSW5kZXggPCBhdHRyaWJ1dGVDb3VudDsgKythdHRyaWJ1 dGVJbmRleCkgewogICAgICAgICB1bnNpZ25lZCBhdHRyaWJ1dGVNaW5pbXVtID0gbWluaW11bVJl cXVpcmVkUmVnaXN0ZXJDb3VudEZvckF0dHJpYnV0ZUZpbHRlciArIGJhY2t0cmFja2luZ1JlZ2lz dGVyUmVxdWlyZW1lbnRzOwo= 238633 2014-09-24 18:28:03 -0700 2014-09-25 17:29:49 -0700 Patch bug-136936-20140925102902.patch text/plain 3106 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMTczOTM5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzc2YjYxMzBlNjQyY2E5 MzY2Zjc4MDg2MTJkNWEzZGIxNDM2OWZlNS4uMTg4YjI3ODFhYjVjY2FmZWVkOTIzMzc3ZWEzYzIz NzA5ZDc5Yzg5MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE0LTA5LTI0ICBZdXN1 a2UgU3V6dWtpICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgorCisgICAgICAgIENTUyBKSVQ6IERv bid0IGFkZCBiYWNrdHJhY2tpbmcgcmVnaXN0ZXIgY291bnQgdG8gbWluaW11bSByZWdpc3RlciBj b3VudCBpZiBpdCdzIG5vdCBuZWNlc3NhcnkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEzNjkzNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIE5vdyBgbWluaW11bVJlcXVpcmVkUmVnaXN0ZXJDb3VudGAgYWx3 YXlzIGNvbnRhaW5zIGBCYWNrdHJhY2tpbmdSZWdpc3RlcmAuCisgICAgICAgIEhvd2V2ZXIgd2hl biB0aGUgZnJhZ21lbnQgaXMgbm90IGluc2lkZSBJbkNoYWluV2l0aERlc2NlbmRhbnRUYWlsLAor ICAgICAgICB0aGlzIGJhY2t0cmFja2luZyByZWdpc3RlciBpcyBub3QgYWN0dWFsbHkgYWxsb2Nh dGVkLgorICAgICAgICBTbyBhZGRpbmcgYGJhY2t0cmFja2luZ1JlZ2lzdGVyUmVxdWlyZW1lbnRz YCBvbmx5IGlmIGl0J3MgbmVjZXNzYXJ5CisgICAgICAgIHRvIHJlZHVjZSB0aGUgdW5uZWNlc3Nh cnkgcmVnaXN0ZXIgcHJlc3N1cmUuCisKKyAgICAgICAgKiBjc3NqaXQvU2VsZWN0b3JDb21waWxl ci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTZWxlY3RvckNvbXBpbGVyOjptaW5pbXVtUmVnaXN0 ZXJSZXF1aXJlbWVudHMpOgorCiAyMDE0LTA5LTI0ICBCcmlhbiBKLiBCdXJnICA8YnVyZ0Bjcy53 YXNoaW5ndG9uLmVkdT4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBzdWJ0cmFjdCBlbGFwc2Vk IHRpbWUgd2hpbGUgZGVidWdnZXIgaXMgcGF1c2VkIGZyb20gcHJvZmlsZSBub2RlcwpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvY3Nzaml0L1NlbGVjdG9yQ29tcGlsZXIuY3BwIGIvU291cmNl L1dlYkNvcmUvY3Nzaml0L1NlbGVjdG9yQ29tcGlsZXIuY3BwCmluZGV4IDQyNGExN2U4YWEzMGE2 YmE0ZWZjZjM4NGQwNjA1ODA1YTNhYzIwNjUuLjMxOWIxZWJlZTMzMmZkZDdjM2ZmMDk0OGM1ODYw NWE3Zjk0MzBlYTQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2Nzc2ppdC9TZWxlY3RvckNv bXBpbGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9jc3NqaXQvU2VsZWN0b3JDb21waWxlci5j cHAKQEAgLTg0NiwyMSArODQ2LDIxIEBAIHN0YXRpYyBpbmxpbmUgYm9vbCBhdHRyaWJ1dGVWYWx1 ZVRlc3RpbmdSZXF1aXJlc0Nhc2VGb2xkaW5nUmVnaXN0ZXIoY29uc3QgQXR0cmliCiB9CiAKIC8v IFN0cmljdCBtaW5pbXVtIHRvIG1hdGNoIGFueXRoaW5nIGludGVyZXN0aW5nOgotLy8gRWxlbWVu dCArIEJhY2t0cmFja2luZ1JlZ2lzdGVyICsgRWxlbWVudERhdGEgKyBhIHBvaW50ZXIgdG8gdmFs dWVzICsgYW4gaW5kZXggb24gdGhhdCBwb2ludGVyICsgdGhlIHZhbHVlIHdlIGV4cGVjdDsKLXN0 YXRpYyBjb25zdCB1bnNpZ25lZCBtaW5pbXVtUmVxdWlyZWRSZWdpc3RlckNvdW50ID0gNjsKKy8v IEVsZW1lbnQgKyBFbGVtZW50RGF0YSArIGEgcG9pbnRlciB0byB2YWx1ZXMgKyBhbiBpbmRleCBv biB0aGF0IHBvaW50ZXIgKyB0aGUgdmFsdWUgd2UgZXhwZWN0Oworc3RhdGljIGNvbnN0IHVuc2ln bmVkIG1pbmltdW1SZXF1aXJlZFJlZ2lzdGVyQ291bnQgPSA1OwogLy8gRWxlbWVudCArIEVsZW1l bnREYXRhICsgc2NyYXRjaFJlZ2lzdGVyICsgYXR0cmlidXRlQXJyYXlQb2ludGVyICsgZXhwZWN0 ZWRMb2NhbE5hbWUgKyAocXVhbGlmaWVkTmFtZUltcGwgJiYgZXhwZWN0ZWRWYWx1ZSkuCiBzdGF0 aWMgY29uc3QgdW5zaWduZWQgbWluaW11bVJlcXVpcmVkUmVnaXN0ZXJDb3VudEZvckF0dHJpYnV0 ZUZpbHRlciA9IDY7CiAKIHN0YXRpYyBpbmxpbmUgdW5zaWduZWQgbWluaW11bVJlZ2lzdGVyUmVx dWlyZW1lbnRzKGNvbnN0IFNlbGVjdG9yRnJhZ21lbnQmIHNlbGVjdG9yRnJhZ21lbnQpCiB7Ci0g ICAgdW5zaWduZWQgbWluaW11bSA9IG1pbmltdW1SZXF1aXJlZFJlZ2lzdGVyQ291bnQ7Ci0gICAg Y29uc3QgVmVjdG9yPEF0dHJpYnV0ZU1hdGNoaW5nSW5mbz4mIGF0dHJpYnV0ZXMgPSBzZWxlY3Rv ckZyYWdtZW50LmF0dHJpYnV0ZXM7Ci0KICAgICB1bnNpZ25lZCBiYWNrdHJhY2tpbmdSZWdpc3Rl clJlcXVpcmVtZW50cyA9IDA7CiAgICAgaWYgKHNlbGVjdG9yRnJhZ21lbnQuYmFja3RyYWNraW5n RmxhZ3MgJiBCYWNrdHJhY2tpbmdGbGFnOjpJbkNoYWluV2l0aERlc2NlbmRhbnRUYWlsKQogICAg ICAgICBiYWNrdHJhY2tpbmdSZWdpc3RlclJlcXVpcmVtZW50cyA9IDE7IC8vIElmIHRoZXJlIGlz IGEgRGVzY2VuZGFudFRhaWwsIHRoZXJlIGlzIGEgYmFja3RyYWNraW5nIHJlZ2lzdGVyLgogCisg ICAgdW5zaWduZWQgbWluaW11bSA9IG1pbmltdW1SZXF1aXJlZFJlZ2lzdGVyQ291bnQgKyBiYWNr dHJhY2tpbmdSZWdpc3RlclJlcXVpcmVtZW50czsKKwogICAgIC8vIEF0dHJpYnV0ZXMgY2F1c2Ug c29tZSByZWdpc3RlciBwcmVzc3VyZS4KKyAgICBjb25zdCBWZWN0b3I8QXR0cmlidXRlTWF0Y2hp bmdJbmZvPiYgYXR0cmlidXRlcyA9IHNlbGVjdG9yRnJhZ21lbnQuYXR0cmlidXRlczsKICAgICB1 bnNpZ25lZCBhdHRyaWJ1dGVDb3VudCA9IGF0dHJpYnV0ZXMuc2l6ZSgpOwogICAgIGZvciAodW5z aWduZWQgYXR0cmlidXRlSW5kZXggPSAwOyBhdHRyaWJ1dGVJbmRleCA8IGF0dHJpYnV0ZUNvdW50 OyArK2F0dHJpYnV0ZUluZGV4KSB7CiAgICAgICAgIHVuc2lnbmVkIGF0dHJpYnV0ZU1pbmltdW0g PSBtaW5pbXVtUmVxdWlyZWRSZWdpc3RlckNvdW50Rm9yQXR0cmlidXRlRmlsdGVyICsgYmFja3Ry YWNraW5nUmVnaXN0ZXJSZXF1aXJlbWVudHM7Cg==