Bug 159859

Summary: DFG CSE should know how to decay a MultiGetByOffset
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 159858    
Bug Blocks:    
Attachments:
Description Flags
the patch keith_miller: review+

Filip Pizlo
Reported 2016-07-16 15:37:52 PDT
This probably means teaching Node::remove() how to decay a MultiGetByOffset, which implies passing Graph& to remove() so that it can create a StructureSet.
Attachments
the patch (7.95 KB, patch)
2018-04-24 08:48 PDT, Filip Pizlo 		keith_miller: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2016-10-23 12:29:04 PDT
Is MultiGetByOffset not dominated by a StructureCheck the way GetByOffset is?
Filip Pizlo
Comment 2 2016-10-23 13:02:26 PDT
(In reply to comment #1) > Is MultiGetByOffset not dominated by a StructureCheck the way GetByOffset is? It's not dominated by a CheckStructure. The MultiGetByOffset does its own structure check. We want one switch statement that both checks that we have the structure we want and loads the value. CheckStructure with multiple structures in its set will be a switch and MultiGetByOffset is always a switch (unless we failed to strength-reduce a one-case MultiGetByOffset), so that would be two switches. That's not cool, because we can't guarantee that B3 will jump-thread or tail-dup two switches in a row. So, MultiGetByOffset is its own check.
Saam Barati
Comment 3 2016-10-28 02:31:48 PDT
(In reply to comment #2) > (In reply to comment #1) > > Is MultiGetByOffset not dominated by a StructureCheck the way GetByOffset is? > > It's not dominated by a CheckStructure. The MultiGetByOffset does its own > structure check. > > We want one switch statement that both checks that we have the structure we > want and loads the value. CheckStructure with multiple structures in its > set will be a switch and MultiGetByOffset is always a switch (unless we > failed to strength-reduce a one-case MultiGetByOffset), so that would be two > switches. That's not cool, because we can't guarantee that B3 will > jump-thread or tail-dup two switches in a row. So, MultiGetByOffset is its > own check. Gotcha. I'll work on fixing this.
Filip Pizlo
Comment 4 2018-04-24 08:48:50 PDT
Created attachment 338653 [details] the patch
Keith Miller
Comment 5 2018-04-24 11:49:59 PDT
Comment on attachment 338653 [details] the patch r=me.
Filip Pizlo
Comment 6 2018-04-24 11:55:02 PDT
Landed in https://trac.webkit.org/changeset/230964/webkit
Radar WebKit Bug Importer
Comment 7 2018-04-24 11:56:26 PDT
<rdar://problem/39692355>
Note You need to log in before you can comment on or make changes to this bug.