Bug 151246

Summary:

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

Product:

WebKit

Reporter:

Benjamin Poulain <benjamin>

Component:

New Bugs

Assignee:

Benjamin Poulain <benjamin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, fpizlo, ggaren, keith_miller, mark.lam, msaboff, saam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

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. ***