| Summary: | REGRESSION(r225913): about 30 JSC test failures on ARMv7 | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
| Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, ews-watchlist, keith_miller, mark.lam, mcatanzaro, msaboff, ossy, rniwa, saam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 108645 | ||||||||
| Attachments: |
|
||||||||
|
Description
Zan Dobersek
2017-12-26 08:56:46 PST
Created attachment 330195 [details]
Patch
Comment on attachment 330195 [details] Patch Attachment 330195 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5836010 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html Created attachment 330199 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 330195 [details] Patch Clearing flags on attachment: 330195 Committed r226298: <https://trac.webkit.org/changeset/226298> All reviewed patches have been landed. Closing bug. Comment on attachment 330195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330195&action=review > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7638 > // X86 only has 6 GP registers, which is not enough for the fast case here. At least without custom code, which is not currently worth the extra code maintenance. > - if (!isX86() || isX86_64()) { > + if (isARM64() || isX86_64()) { The code change invalidates the comment; so it should have updated the comment. Also seems slightly wrong to require anyone adding a future 64-bit target to this if statement. The old code was X86-specific so would do no harm for non-X86. The new code means we only get this optimization for explicitly listed platforms. Would be slightly nicer if this could be written in a more direct way, like if (!tooFewGPRegisters()). Reopening for Zan... I should have spotted that the comment was stale, sorry. At least that should be fixed. Comment on attachment 330195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330195&action=review >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7638 >> + if (isARM64() || isX86_64()) { > > The code change invalidates the comment; so it should have updated the comment. > > Also seems slightly wrong to require anyone adding a future 64-bit target to this if statement. The old code was X86-specific so would do no harm for non-X86. The new code means we only get this optimization for explicitly listed platforms. Would be slightly nicer if this could be written in a more direct way, like if (!tooFewGPRegisters()). I think you can just do `if (is64Bit())`. That's assuming all 64 bit platforms have enough registers for this, which I think is the case. Modified the comment and the if-statement condition in r226616. https://trac.webkit.org/r226616 |