Bug 151246 - [JSC] Add support for the extra registers that can be clobbered by Specials
Summary: [JSC] Add support for the extra registers that can be clobbered by Specials
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
: 151146 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-11-12 21:17 PST by Benjamin Poulain
Modified: 2015-11-16 16:49 PST (History)
7 users (show)

See Also:


Attachments
Patch (23.21 KB, patch)
2015-11-12 21:25 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (23.38 KB, patch)
2015-11-13 13:44 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (24.18 KB, patch)
2015-11-16 16:39 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2015-11-12 21:17:42 PST
[JSC] Add support for the extra registers that can be clobbered by any Special
Comment 1 Benjamin Poulain 2015-11-12 21:25:38 PST
Created attachment 265467 [details]
Patch
Comment 2 WebKit Commit Bot 2015-11-12 21:28:30 PST
Attachment 265467 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/air/AirInst.h:131:  The parameter name "functor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2683:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2684:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2696:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2727:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2728:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2744:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2761:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2762:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2774:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2798:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2799:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
Total errors found: 12 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Benjamin Poulain 2015-11-13 13:44:12 PST
Created attachment 265498 [details]
Patch
Comment 4 WebKit Commit Bot 2015-11-13 13:46:12 PST
Attachment 265498 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/air/AirInst.h:131:  The parameter name "functor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2683:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2684:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2696:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2727:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2728:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2744:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2761:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2762:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2774:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2798:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2799:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
Total errors found: 12 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Geoffrey Garen 2015-11-14 12:17:39 PST
Comment on attachment 265498 [details]
Patch

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

R=me

> Source/JavaScriptCore/b3/air/AirInst.h:131
> +    void forEachExtraClobberedTmps(Arg::Type, const Functor& functor);

I would say ClobberedTmp instead of Tmps since each is singular.

> Source/JavaScriptCore/b3/air/AirInstInlines.h:98
> +    const RegisterSet& clobberedRegisters = extraClobberedRegs();

Let's standardize on tmp or reg, but not both.

> Source/JavaScriptCore/b3/air/AirInstInlines.h:99
> +    clobberedRegisters.forEach([functor, type] (Reg reg) {

Hmmm... Ignore this comment.

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:153
> +                // All the Def()s interfere with eachother.

Each other
Comment 6 Geoffrey Garen 2015-11-16 13:00:37 PST
Comment on attachment 265498 [details]
Patch

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

> Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp:148
>          inst.forEachTmp([&] (Tmp& arg, Arg::Role role, Arg::Type argType) {

Maybe you should consider a whole new function that combines forEachTmp and forEachExtra.
Comment 7 Benjamin Poulain 2015-11-16 16:39:54 PST
Created attachment 265638 [details]
Patch
Comment 8 WebKit Commit Bot 2015-11-16 16:43:26 PST
Attachment 265638 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/air/AirInst.h:131:  The parameter name "functor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2777:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2778:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2790:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2821:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2822:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2838:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2855:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2856:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2868:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2892:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:2893:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
ERROR: Source/JavaScriptCore/b3/air/AirSpillEverything.cpp:61:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 13 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Benjamin Poulain 2015-11-16 16:44:27 PST
Comment on attachment 265638 [details]
Patch

Clearing flags on attachment: 265638

Committed r192497: <http://trac.webkit.org/changeset/192497>
Comment 10 Benjamin Poulain 2015-11-16 16:44:30 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Benjamin Poulain 2015-11-16 16:49:23 PST
*** Bug 151146 has been marked as a duplicate of this bug. ***