Bug 202833

Summary: GenerateAndAllocateRegisters can trivially elide self moves at end of liveness
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, mark.lam, msaboff, saam, tsavell, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Keith Miller
Reported 2019-10-10 17:24:33 PDT
GenerateAndAllocateRegisters can trivially elide self moves at end of liveness
Attachments
Patch (5.22 KB, patch)
2019-10-10 17:27 PDT, Keith Miller
no flags
Keith Miller
Comment 1 2019-10-10 17:27:53 PDT
Saam Barati
Comment 2 2019-10-10 18:07:24 PDT
Comment on attachment 380708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380708&action=review > Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:54 > + if (!reg) > + return; should we assert here that nothing in m_currentAllocation points to tmp?
Keith Miller
Comment 3 2019-10-10 18:35:23 PDT
Comment on attachment 380708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380708&action=review >> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:54 >> + return; > > should we assert here that nothing in m_currentAllocation points to tmp? My intention was that would be handled in the lower loop. I suppose the lower loop could say a disallowed register is allocated to tmp. But that seems very unlikely and probably wouldn’t break anything anyway. Maybe I’m missing something though?
Keith Miller
Comment 4 2019-10-10 18:38:50 PDT
Mac debug builder just seems to be down...
Saam Barati
Comment 5 2019-10-10 18:39:42 PDT
Comment on attachment 380708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380708&action=review >>> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:54 >>> + return; >> >> should we assert here that nothing in m_currentAllocation points to tmp? > > My intention was that would be handled in the lower loop. I suppose the lower loop could say a disallowed register is allocated to tmp. But that seems very unlikely and probably wouldn’t break anything anyway. Maybe I’m missing something though? nope, it's handled. Ignore me.
WebKit Commit Bot
Comment 6 2019-10-10 19:23:44 PDT
Comment on attachment 380708 [details] Patch Clearing flags on attachment: 380708 Committed r250999: <https://trac.webkit.org/changeset/250999>
WebKit Commit Bot
Comment 7 2019-10-10 19:23:45 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2019-10-10 19:24:16 PDT
Truitt Savell
Comment 9 2019-10-15 11:03:15 PDT
It looks like the changes in https://trac.webkit.org/changeset/250999/webkit has caused 3 tests to time out on Mac Debug: wasm/iframe-parent-postmessage.html wasm/iframe-postmessage.html wasm/window-postmessage.html Not these tests are taking 25 seconds to pass, or they time out. before they were taking only a few seconds to pass. I was able to reproduce this locally by running the three tests in iterations.
Keith Miller
Comment 10 2019-10-16 12:27:52 PDT
(In reply to Truitt Savell from comment #9) > It looks like the changes in https://trac.webkit.org/changeset/250999/webkit > > has caused 3 tests to time out on Mac Debug: > wasm/iframe-parent-postmessage.html > wasm/iframe-postmessage.html > wasm/window-postmessage.html > > Not these tests are taking 25 seconds to pass, or they time out. before they > were taking only a few seconds to pass. I was able to reproduce this locally > by running the three tests in iterations. Opened https://bugs.webkit.org/show_bug.cgi?id=203050 with a fix.
Note You need to log in before you can comment on or make changes to this bug.