Bug 151246 – [JSC] Add support for the extra registers that can be clobbered by Specials

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[JSC] Add support for the extra registers that can be clobbered by any Special

[Benjamin Poulain]

Created attachment 265467 [details]
Patch

[WebKit Commit Bot]

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.

[Benjamin Poulain]

Created attachment 265498 [details]
Patch

[WebKit Commit Bot]

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.

[Geoffrey Garen]

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

[Geoffrey Garen]

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.

[Benjamin Poulain]

Created attachment 265638 [details]
Patch

[WebKit Commit Bot]

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.

[Benjamin Poulain]

Comment on attachment 265638 [details]
Patch

Clearing flags on attachment: 265638

Committed r192497: <http://trac.webkit.org/changeset/192497>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.

[Benjamin Poulain]

*** Bug 151146 has been marked as a duplicate of this bug. ***