Bug 67551 - DFG JIT speculation failure does recovery of additions without reboxing
Summary: DFG JIT speculation failure does recovery of additions without reboxing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-02 20:48 PDT by Filip Pizlo
Modified: 2011-09-06 13:08 PDT (History)
2 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 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.
Comment 1 Filip Pizlo 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.
Comment 2 Filip Pizlo 2011-09-02 21:15:59 PDT
Created attachment 106243 [details]
the patch
Comment 3 WebKit Review Bot 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.
Comment 4 Sam Weinig 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.
Comment 5 Filip Pizlo 2011-09-02 21:30:32 PDT
Created attachment 106244 [details]
the patch - removed tabs
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2011-09-02 22:22:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Geoffrey Garen 2011-09-06 13:03:08 PDT
Could this patch have included a regression test?
Comment 9 Filip Pizlo 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.