Bug 67551

Summary:

DFG JIT speculation failure does recovery of additions without reboxing

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ggaren, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
the patch	sam: review+
the patch - removed tabs	none

Filip Pizlo

Reported 2011-09-02 20:48:53 PDT

The DFG JIT speculation failure code can undo additions - so if we realize that we executed a destructive addition incorrectly, we can revert it. But the code does not work: it performs an addition on the wrong register (it reverses the source and destination) and then fails to rebox the result, if the destructive addition also did implicit unboxing via zero extension.

Attachments
the patch (1.82 KB, patch) 2011-09-02 21:15 PDT, Filip Pizlo	sam: review+	Details Formatted Diff Diff
the patch - removed tabs (1.84 KB, patch) 2011-09-02 21:30 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2011-09-02 21:02:19 PDT

Correction: the recovery is done in the right order. The bug here is that it does not do reboxing.

Filip Pizlo

Comment 2 2011-09-02 21:15:59 PDT

Created attachment 106243 [details] the patch

WebKit Review Bot

Comment 3 2011-09-02 21:17:48 PDT

Attachment 106243 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/ChangeLog:3: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/ChangeLog:4: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/ChangeLog:5: Line contains tab character. [whitespace/tab] [5] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sam Weinig

Comment 4 2011-09-02 21:27:12 PDT

Comment on attachment 106243 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=106243&action=review > Source/JavaScriptCore/ChangeLog:6 > + Please remove tabs.

Filip Pizlo

Comment 5 2011-09-02 21:30:32 PDT

Created attachment 106244 [details] the patch - removed tabs

WebKit Review Bot

Comment 6 2011-09-02 22:22:41 PDT

Comment on attachment 106244 [details] the patch - removed tabs Clearing flags on attachment: 106244 Committed r94478: <http://trac.webkit.org/changeset/94478>

WebKit Review Bot

Comment 7 2011-09-02 22:22:45 PDT

All reviewed patches have been landed. Closing bug.

Geoffrey Garen

Comment 8 2011-09-06 13:03:08 PDT

Could this patch have included a regression test?

Filip Pizlo

Comment 9 2011-09-06 13:08:09 PDT

(In reply to comment #8) > Could this patch have included a regression test? It could have; at the time I wasn't sure if I could even repro it without tiered compilation but now I know I can. A test case is on the way.

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