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+

Description Filip Pizlo 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!
Comment 1 Filip Pizlo 2017-08-11 14:15:32 PDT
Created attachment 317960 [details]
I think this should work
Comment 2 Filip Pizlo 2017-08-11 17:34:53 PDT
Created attachment 317977 [details]
maybe the patch
Comment 3 Filip Pizlo 2017-08-11 18:03:13 PDT
Created attachment 317979 [details]
the patch
Comment 4 Mark Lam 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/.
Comment 5 Filip Pizlo 2017-08-12 11:45:12 PDT
Landed in https://trac.webkit.org/changeset/220625/webkit
Comment 6 Radar WebKit Bug Importer 2017-08-12 11:46:03 PDT
<rdar://problem/33864295>