Bug 197305 – [Air] highOrderAdjacents in AbstractColoringAllocator::conservativeHeuristic should be some kind of array

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Robin Morisset]

Currently it is a HashSet, but it only ever holds at most registerCount() items. And linear search tends to be faster on such a small collection than hashing + searching in a HashSet.

Further benefits include avoiding the allocation of the HashSet, not actually adding the nodes adjacent to V (since there are no duplicates in the adjacency lists).

[Robin Morisset]

Created attachment 368297 [details]
Patch

Approximately a 6% speedup for the register allocator.

[EWS Watchlist]

Attachment 368297 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:234:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Robin Morisset]

Created attachment 368334 [details]
Patch

Fix style nits.

[Robin Morisset]

Created attachment 368998 [details]
Patch

Just rebased.

[EWS Watchlist]

Comment on attachment 368998 [details]
Patch

Attachment 368998 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12093904

New failing tests:
http/tests/css/filters-on-iframes.html
svg/dynamic-updates/SVGRadialGradientElement-svgdom-href-prop.html

[EWS Watchlist]

Created attachment 369031 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Keith Miller]

Comment on attachment 368998 [details]
Patch

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

r=me

> Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:195
> +        Vector<IndexType, 100> highOrderAdjacents;

Why not just make this MacroAssembler::numGPRs + MacroAssembler::numFPRs? Alternatively you could make registerCount() constexpr.

[Robin Morisset]

Comment on attachment 368998 [details]
Patch

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

>> Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:195
>> +        Vector<IndexType, 100> highOrderAdjacents;
> 
> Why not just make this MacroAssembler::numGPRs + MacroAssembler::numFPRs? Alternatively you could make registerCount() constexpr.

Making registerCount() constexpr is very tricky since it is defined as the length of a vector that is passed through quite a bit of code.
I am currently testing MacroAssembler::numGPRs + MacroAssembler::numFPRs.

[Robin Morisset]

Created attachment 378401 [details]
Patch

Submitting to EWS, I will cq+ it if it passes the tests.

[WebKit Commit Bot]

Comment on attachment 378401 [details]
Patch

Clearing flags on attachment: 378401

Committed r249677: <https://trac.webkit.org/changeset/249677>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55205490>