Bug 162749

Summary: Air should be able to replace constant materializations with adds
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=162764
Attachments:
Description Flags
the patch
none
the patch
none
the patch ysuzuki: review+

Filip Pizlo
Reported 2016-09-29 14:03:13 PDT
We thought this was a good idea for a long time but then we got evidence that it's not a good idea. In the meantime, I implemented it. I will use this bug to stash the code and report any results.
Attachments
the patch (2.14 KB, patch)
2016-09-29 14:03 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (3.12 KB, patch)
2016-10-10 20:00 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (4.50 KB, patch)
2016-10-10 20:15 PDT, Filip Pizlo 		ysuzuki: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2016-09-29 14:03:49 PDT
Created attachment 290241 [details] the patch This turns out to be very easy to write.
Filip Pizlo
Comment 2 2016-10-10 20:00:25 PDT
Created attachment 291216 [details] the patch
Filip Pizlo
Comment 3 2016-10-10 20:15:25 PDT
Created attachment 291218 [details] the patch
Yusuke Suzuki
Comment 4 2016-10-11 13:47:39 PDT
Comment on attachment 291218 [details] the patch r=me
Filip Pizlo
Comment 5 2016-10-11 13:54:47 PDT
Landed in https://trac.webkit.org/changeset/207164
Saam Barati
Comment 6 2016-10-11 14:36:41 PDT
Comment on attachment 291218 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=291218&action=review > Source/JavaScriptCore/b3/air/AirFixObviousSpills.cpp:195 > + // First handle some special instructions. Nit: I don't think this comment really adds anything. > Source/JavaScriptCore/b3/air/AirFixObviousSpills.cpp:210 > + // should add fancier materializations here for ARM if the BigImm is yuge. s/yuge/huge Maybe open a bug for this?
Note You need to log in before you can comment on or make changes to this bug.