WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
150762
B3 should have a Select opcode
https://bugs.webkit.org/show_bug.cgi?id=150762
Summary
B3 should have a Select opcode
Filip Pizlo
Reported
2015-10-31 16:50:24 PDT
Because conditional moves are pretty neat.
Attachments
work in progress
(22.36 KB, patch)
2015-11-20 11:50 PST
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(39.63 KB, patch)
2015-11-20 13:28 PST
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(44.55 KB, patch)
2015-11-20 13:58 PST
,
Filip Pizlo
benjamin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2015-11-20 11:50:26 PST
Created
attachment 265976
[details]
work in progress
Filip Pizlo
Comment 2
2015-11-20 13:28:21 PST
Created
attachment 265988
[details]
the patch
WebKit Commit Bot
Comment 3
2015-11-20 13:30:37 PST
Attachment 265988
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:847: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/B3ValueKey.h:65: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4751: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4752: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4753: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4754: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4770: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4771: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4772: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4773: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4792: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4793: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4794: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4795: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4796: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/B3Value.h:238: Missing space before { [whitespace/braces] [5] ERROR: Source/JavaScriptCore/b3/B3Value.h:248: Missing space before { [whitespace/braces] [5] Total errors found: 17 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 4
2015-11-20 13:58:19 PST
Created
attachment 265995
[details]
the patch
WebKit Commit Bot
Comment 5
2015-11-20 14:00:47 PST
Attachment 265995
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/b3/B3Value.h:238: Missing space before { [whitespace/braces] [5] ERROR: Source/JavaScriptCore/b3/B3Value.h:248: Missing space before { [whitespace/braces] [5] ERROR: Source/JavaScriptCore/b3/B3ValueKey.h:65: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4751: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4752: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4753: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4754: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4770: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4771: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4772: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4773: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4792: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4793: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4794: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4795: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4796: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4902: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4903: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4904: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:4905: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:847: 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: 21 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Benjamin Poulain
Comment 6
2015-11-20 14:10:21 PST
Comment on
attachment 265995
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=265995&action=review
> Source/JavaScriptCore/b3/B3ReduceStrength.cpp:555 > + && m_value->child(0)->child(1)->isInt32(1)
Should be ->asInt() & 1
> Source/JavaScriptCore/dfg/DFGCommon.h:41 > -#define FTL_USES_B3 0 > +#define FTL_USES_B3 1
Oops
Filip Pizlo
Comment 7
2015-11-20 14:31:57 PST
Landed in
http://trac.webkit.org/changeset/192699
Filip Pizlo
Comment 8
2015-11-20 14:35:28 PST
(In reply to
comment #6
)
> Comment on
attachment 265995
[details]
> the patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=265995&action=review
> > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:555 > > + && m_value->child(0)->child(1)->isInt32(1) > > Should be ->asInt() & 1
I think that would be wrong. If you take a boolean value - i.e. 0 or 1 - and xor it with a value that is not 1 but has the 1 bit set, like say 41, then you're not actually doing a logical not.
> > > Source/JavaScriptCore/dfg/DFGCommon.h:41 > > -#define FTL_USES_B3 0 > > +#define FTL_USES_B3 1 > > Oops
Fixed!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug