Bug 159859

Summary:

DFG CSE should know how to decay a MultiGetByOffset

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

159858

Bug Blocks:

Attachments:

Description	Flags
the patch	keith_miller: review+

Description Filip Pizlo 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.

Comment 1 Saam Barati 2016-10-23 12:29:04 PDT

Is MultiGetByOffset not dominated by a StructureCheck the way GetByOffset is?

Comment 2 Filip Pizlo 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.

Comment 3 Saam Barati 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.

Comment 4 Filip Pizlo 2018-04-24 08:48:50 PDT

Created attachment 338653 [details]
the patch

Comment 5 Keith Miller 2018-04-24 11:49:59 PDT

Comment on attachment 338653 [details]
the patch

r=me.

Comment 6 Filip Pizlo 2018-04-24 11:55:02 PDT

Landed in https://trac.webkit.org/changeset/230964/webkit

Comment 7 Radar WebKit Bug Importer 2018-04-24 11:56:26 PDT

<rdar://problem/39692355>