Bug 175483

Summary: Caging shouldn't have to use a patchpoint for adding
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 175493    
Attachments:
Description Flags
I think this should work
none
maybe the patch
none
the patch mark.lam: review+

Filip Pizlo
Reported 2017-08-11 10:24:23 PDT
Currently it's worth 1.6% on Octane and 5% on Kraken to use a patchpoint there. That's super weird!
Attachments
I think this should work (7.13 KB, patch)
2017-08-11 14:15 PDT, Filip Pizlo
no flags
maybe the patch (14.02 KB, patch)
2017-08-11 17:34 PDT, Filip Pizlo
no flags
the patch (16.49 KB, patch)
2017-08-11 18:03 PDT, Filip Pizlo
mark.lam: review+
Filip Pizlo
Comment 1 2017-08-11 14:15:32 PDT
Created attachment 317960 [details] I think this should work
Filip Pizlo
Comment 2 2017-08-11 17:34:53 PDT
Created attachment 317977 [details] maybe the patch
Filip Pizlo
Comment 3 2017-08-11 18:03:13 PDT
Created attachment 317979 [details] the patch
Mark Lam
Comment 4 2017-08-12 07:51:34 PDT
Comment on attachment 317979 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=317979&action=review r=me > Source/JavaScriptCore/ChangeLog:18 > + hoist it - that would require quite a bit of algebraic reasoning. But this is the only case we know > + where our current constant reassociation heuristics are wrong is caging. So, we can get away with /this is the only case we know where/the only case we know of where/.
Filip Pizlo
Comment 5 2017-08-12 11:45:12 PDT
Radar WebKit Bug Importer
Comment 6 2017-08-12 11:46:03 PDT
Note You need to log in before you can comment on or make changes to this bug.