Bug 64068

Summary: DFG JIT unnecessarily boxes and unboxes values during silent spilling
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
barraclough: review-
the patch (fix review) none

Filip Pizlo
Reported 2011-07-07 00:08:25 PDT
DFG silent spilling is performed for guarded slow-path C function calls. There is a one-to-one relationship between each silent spill and silent fill. Hence, there is no need to box and unbox values, yet the DFG JIT boxes and unboxes anyway.
Attachments
the patch (11.00 KB, patch)
2011-07-07 00:12 PDT, Filip Pizlo 		barraclough: review-
DetailsFormatted DiffDiff
the patch (fix review) (12.69 KB, patch)
2011-07-13 23:45 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2011-07-07 00:12:11 PDT
Created attachment 99951 [details] the patch
Gavin Barraclough
Comment 2 2011-07-07 15:45:40 PDT
Comment on attachment 99951 [details] the patch Hi Filip, I think there may be a subtle bug in this code. When a register is silently spilled it is only actually written out to memory if it needs to be spilled (hasn't already been). This means that if you have values in registers XMM0 & XMM1, and XMM0 has already been spilled as a boxed value, then after the silent spill one of the values in memory will be boxed and the other won't. As such, you need to handle this in the silentFillFPR. If the register needsSpill() then it is unboxed, if not the value in memory is boxed. Also, please expand on the ChangeLog entry.
Filip Pizlo
Comment 3 2011-07-13 23:45:46 PDT
Created attachment 100776 [details] the patch (fix review)
Gavin Barraclough
Comment 4 2011-07-13 23:52:17 PDT
Comment on attachment 100776 [details] the patch (fix review) Looks great!
WebKit Review Bot
Comment 5 2011-07-14 10:32:25 PDT
Comment on attachment 100776 [details] the patch (fix review) Clearing flags on attachment: 100776 Committed r91010: <http://trac.webkit.org/changeset/91010>
WebKit Review Bot
Comment 6 2011-07-14 10:32:30 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.