Bug 151556

Summary: [JSC] Speed up Air Liveness Analysis on Tmps
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, commit-queue, fpizlo, ggaren, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch fpizlo: review+

Benjamin Poulain
Reported 2015-11-22 17:05:10 PST
[JSC] Speed up Air Liveness Analysis on Tmps
Attachments
Patch (53.20 KB, patch)
2015-11-22 17:34 PST, Benjamin Poulain 		no flags
DetailsFormatted DiffDiff
Patch (68.13 KB, patch)
2015-11-30 17:42 PST, Benjamin Poulain 		fpizlo: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Benjamin Poulain
Comment 1 2015-11-22 17:34:24 PST
Created attachment 266074 [details] Patch
WebKit Commit Bot
Comment 2 2015-11-22 17:35:35 PST
Attachment 266074 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/air/AirReportUsedRegisters.cpp:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/b3/air/AirTmpLiveness.h:53: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirTmpLiveness.h:197: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirTmpLiveness.h:206: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirTmpLiveness.h:216: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 5 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 3 2015-11-29 18:24:15 PST
Why not do the optimization in Air::Liveness and make it also work for StackSlots? Those also have indices.
Benjamin Poulain
Comment 4 2015-11-29 22:03:10 PST
(In reply to comment #3) > Why not do the optimization in Air::Liveness and make it also work for > StackSlots? Those also have indices. But they don't have types right? The way things are split in TmpLiveness is how it is best consumed by the allocator. I may use the absolute indices directly in the future too.
Filip Pizlo
Comment 5 2015-11-30 08:32:51 PST
(In reply to comment #4) > (In reply to comment #3) > > Why not do the optimization in Air::Liveness and make it also work for > > StackSlots? Those also have indices. > > But they don't have types right? The way things are split in TmpLiveness is > how it is best consumed by the allocator. > > I may use the absolute indices directly in the future too. But you could have Liveness take a traits or adaptor or something as a template argument that produces an index given a value of vice-versa. For temps, you'd have two versions of the adapter - one for FP and one for GP.
Benjamin Poulain
Comment 6 2015-11-30 17:42:11 PST
Created attachment 266309 [details] Patch
WebKit Commit Bot
Comment 7 2015-11-30 17:44:56 PST
Attachment 266309 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/air/AirLiveness.h:92: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirLiveness.h:218: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirLiveness.h:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/air/AirLiveness.h:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 4 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 8 2015-11-30 18:08:55 PST
Comment on attachment 266309 [details] Patch R=me assuming you also fix the bug you found in the extra early clobbers.
Benjamin Poulain
Comment 9 2015-11-30 18:27:07 PST
Committed r192851: <http://trac.webkit.org/changeset/192851>
Note You need to log in before you can comment on or make changes to this bug.