Bug 67551

Summary: DFG JIT speculation failure does recovery of additions without reboxing
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
sam: review+
the patch - removed tabs none

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.