WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
181162
REGRESSION(
r225913
): about 30 JSC test failures on ARMv7
https://bugs.webkit.org/show_bug.cgi?id=181162
Summary
REGRESSION(r225913): about 30 JSC test failures on ARMv7
Zan Dobersek
Reported
2017-12-26 08:56:46 PST
REGRESSION(
r225913
): about 30 JSC test failures on ARMv7
Attachments
Patch
(1.87 KB, patch)
2017-12-26 09:01 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
(2.64 MB, application/zip)
2017-12-26 10:11 PST
,
EWS Watchlist
no flags
Details
View All
Add attachment
proposed patch, testcase, etc.
Zan Dobersek
Comment 1
2017-12-26 09:01:06 PST
https://trac.webkit.org/changeset/225913
Zan Dobersek
Comment 2
2017-12-26 09:01:19 PST
Created
attachment 330195
[details]
Patch
EWS Watchlist
Comment 3
2017-12-26 10:11:33 PST
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
EWS Watchlist
Comment 4
2017-12-26 10:11:34 PST
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
Zan Dobersek
Comment 5
2017-12-27 02:52:41 PST
Comment on
attachment 330195
[details]
Patch Clearing flags on attachment: 330195 Committed
r226298
: <
https://trac.webkit.org/changeset/226298
>
Zan Dobersek
Comment 6
2017-12-27 02:52:45 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7
2018-01-02 13:17:59 PST
<
rdar://problem/36261349
>
Darin Adler
Comment 8
2018-01-07 16:58:04 PST
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()).
Michael Catanzaro
Comment 9
2018-01-07 17:05:59 PST
Reopening for Zan... I should have spotted that the comment was stale, sorry. At least that should be fixed.
Keith Miller
Comment 10
2018-01-08 16:30:13 PST
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.
Zan Dobersek
Comment 11
2018-01-09 01:33:33 PST
Modified the comment and the if-statement condition in
r226616
.
https://trac.webkit.org/r226616
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug