Bug 137315

Summary: Object allocation sinking should have a sound story for picking materialization points
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, commit-queue, ggaren, mark.lam, mhahnenb, mmirman, msaboff, nrotem, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 137310    
Bug Blocks: 137126, 137168    
Attachments:
Description Flags
work in progress
none
the patch oliver: review+

Filip Pizlo
Reported 2014-10-01 14:46:47 PDT
The current code has roughly the right intuition for picking materialization sites: you need to forward flow the "I was materialized" property. It's true that this is an important input. Where the logic goes wrong is that it then tries to do materializations at the *escaping sites* where an object goes from "not materialized" to "materialized". In the case of loops (and probably some crazy irreducible control flow) you may have a point where an object becomes materialized is not an escaping site. The code should handle that case.
Attachments
work in progress (23.14 KB, patch)
2014-10-02 12:05 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (24.01 KB, patch)
2014-10-02 12:14 PDT, Filip Pizlo 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2014-10-02 12:05:27 PDT
Created attachment 239126 [details] work in progress
Filip Pizlo
Comment 2 2014-10-02 12:14:01 PDT
Created attachment 239127 [details] the patch
WebKit Commit Bot
Comment 3 2014-10-02 12:15:56 PDT
Attachment 239127 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGGraph.h:854: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 4 2014-10-02 12:16:30 PDT
(In reply to comment #3) > Attachment 239127 [details] did not pass style-queue: > > > ERROR: Source/JavaScriptCore/dfg/DFGGraph.h:854: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] > Total errors found: 1 in 11 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. Fixed.
Oliver Hunt
Comment 5 2014-10-02 12:18:39 PDT
Comment on attachment 239127 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=239127&action=review > Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:761 > + Node* result; For my sanity initialize result = nullptr;
Filip Pizlo
Comment 6 2014-10-02 12:23:59 PDT
(In reply to comment #5) > (From update of attachment 239127 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=239127&action=review > > > Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:761 > > + Node* result; > > For my sanity initialize result = nullptr; OK!
Filip Pizlo
Comment 7 2014-10-02 12:38:25 PDT
Landed in http://trac.webkit.org/changeset/174224
Note You need to log in before you can comment on or make changes to this bug.