Bug 151290

Summary: [JSC] Speed up the coalescing-related operation of the iterated register allocator
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 151330    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Benjamin Poulain 2015-11-13 19:57:38 PST
[JSC] Speed up the coalescing-related operation of the iterated register allocator
Comment 1 Benjamin Poulain 2015-11-13 20:22:42 PST
Created attachment 265530 [details]
Patch
Comment 2 Geoffrey Garen 2015-11-14 12:25:41 PST
Comment on attachment 265530 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265530&action=review

r=me

Is this a speedup?

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:760
> +    // in the vector has the "identifier" for the move.

as

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:781
> +    struct OrderedMoveSet {

Can this go in its own file?

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:797
> +            return m_positionInMoveList[index] != std::numeric_limits<unsigned>::max();

Let's use wtf::notFound.

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:831
> +            // This assertion is a bit strict but that how the move list should be used. The only kind of moves that can

that is

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:832
> +            // return to the list are the one that we previous failed to coalesced with the conservative heuristics.

the ones
previously
to coalesce
Comment 3 Benjamin Poulain 2015-11-16 13:53:16 PST
Created attachment 265611 [details]
Patch for landing
Comment 4 Benjamin Poulain 2015-11-16 13:54:43 PST
(In reply to comment #2)
> r=me
> 
> Is this a speedup?

Quite a bit, yep. Globally, it is 2%.

> > Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:781
> > +    struct OrderedMoveSet {
> 
> Can this go in its own file?

If I need a generic version, I'll make a new file.

Here, OrderedMoveSet knows exactly how it is supposed to be used. IMHO, it is better to keep the code close.
Comment 5 Benjamin Poulain 2015-11-16 15:47:09 PST
Comment on attachment 265611 [details]
Patch for landing

Clearing flags on attachment: 265611

Committed r192492: <http://trac.webkit.org/changeset/192492>
Comment 6 Benjamin Poulain 2015-11-16 15:47:13 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 WebKit Commit Bot 2015-11-16 16:10:10 PST
Re-opened since this is blocked by bug 151330