Bug 175483 - Caging shouldn't have to use a patchpoint for adding
Summary: Caging shouldn't have to use a patchpoint for adding
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks: 175493
  Show dependency tree / graph
 
Reported: 2017-08-11 10:24 PDT by Filip Pizlo
Modified: 2017-08-12 11:46 PDT (History)
9 users (show)

See Also:

Attachments
I think this should work (7.13 KB, patch)
2017-08-11 14:15 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
maybe the patch (14.02 KB, patch)
2017-08-11 17:34 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (16.49 KB, patch)
2017-08-11 18:03 PDT, Filip Pizlo 		mark.lam: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>