Bug 151213

Summary:

B3 should be able to compile programs with CheckAdd, CheckSub, and CheckMul

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, benjamin, commit-queue, ggaren, keith_miller, mark.lam, mhahnenb, msaboff, nrotem, oliver, saam, sam

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

151228, 150279, 151214, 151230

Attachments:

Description	Flags
work in progress	none
the patch	none
the patch	none
the patch	benjamin: review+

Filip Pizlo

Reported 2015-11-12 12:42:00 PST

Patch forthcoming.

Attachments
work in progress (8.26 KB, patch) 2015-11-12 12:43 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (11.21 KB, patch) 2015-11-12 15:38 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (13.51 KB, patch) 2015-11-12 15:45 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (45.16 KB, patch) 2015-11-12 16:49 PST, Filip Pizlo	benjamin: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2015-11-12 12:43:32 PST

Created attachment 265423 [details] work in progress

Filip Pizlo

Comment 2 2015-11-12 15:38:10 PST

Created attachment 265443 [details] the patch

WebKit Commit Bot

Comment 3 2015-11-12 15:40:38 PST

Attachment 265443 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:2931: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2932: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2935: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 4 2015-11-12 15:45:32 PST

Created attachment 265444 [details] the patch Improved patch.

WebKit Commit Bot

Comment 5 2015-11-12 15:47:58 PST

Attachment 265444 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:2931: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2932: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2935: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] Total errors found: 3 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 6 2015-11-12 15:59:49 PST

Comment on attachment 265444 [details] the patch Going to add more to this patch.

Filip Pizlo

Comment 7 2015-11-12 16:49:19 PST

Created attachment 265452 [details] the patch

WebKit Commit Bot

Comment 8 2015-11-12 16:52:18 PST

Attachment 265452 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:2931: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2932: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2935: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2967: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2968: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2998: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2999: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3029: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3047: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3056: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3069: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3070: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3073: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3105: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3106: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3141: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3142: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3172: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3190: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3199: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3212: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3213: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3242: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3243: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3276: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3277: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3306: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3307: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3336: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3354: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:3363: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] Total errors found: 31 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Benjamin Poulain

Comment 9 2015-11-12 17:23:03 PST

Comment on attachment 265452 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=265452&action=review Gosh is this cool. > Source/JavaScriptCore/ChangeLog:22 > + Note that I had to convert CheckMul handling to always use a three-operand form. That's because > + there is no other way to tell Air that we need the original value alive. This means that on X86 > + we'll emit an extra Move just before the checked mul. That's probably fine since that's > + necessary anyway. Aww, that sucks. That move seems unavoidable in most cases though.

Filip Pizlo

Comment 10 2015-11-12 17:30:50 PST

Landed in http://trac.webkit.org/changeset/192400

Note You need to log in before you can comment on or make changes to this bug.