Summary: | Function allocation sinking works for wrong reasons | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Basile Clement <basile_clement> | ||||||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, fpizlo, ggaren, mark.lam, msaboff | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Basile Clement
2015-04-24 17:42:52 PDT
Created attachment 251602 [details]
Should do the right thing
Compiles, but untested - will check my understanding of the nodes is correct and does not prevent valid object allocation sinking and add a ChangeLog entry on Monday.
Created attachment 251763 [details]
Seems to be doing the right thing
Created attachment 251764 [details]
Seems to be doing the right thing
Comment on attachment 251764 [details] Seems to be doing the right thing View in context: https://bugs.webkit.org/attachment.cgi?id=251764&action=review > Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:812 > + if (target->op() != NewObject) Unsure if I should add a helper function in DFG::Node for that... You should rebase :-) Comment on attachment 251764 [details] Seems to be doing the right thing View in context: https://bugs.webkit.org/attachment.cgi?id=251764&action=review >> Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:812 >> + if (target->op() != NewObject) > > Unsure if I should add a helper function in DFG::Node for that... Seems wrong - MaterializeNewObject is also a sink candidate. (In reply to comment #6) > Comment on attachment 251764 [details] > Seems to be doing the right thing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251764&action=review > > >> Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:812 > >> + if (target->op() != NewObject) > > > > Unsure if I should add a helper function in DFG::Node for that... > > Seems wrong - MaterializeNewObject is also a sink candidate. Oh, duh, indeed. Fixing that, w/ a helper function then. Created attachment 251768 [details]
MaterializeNewObject is also sinkable...
Sorry, should have tried a clean build. Created attachment 251771 [details]
Remove spurious MustGenerate
Comment on attachment 251771 [details] Remove spurious MustGenerate Clearing flags on attachment: 251771 Committed r183419: <http://trac.webkit.org/changeset/183419> All reviewed patches have been landed. Closing bug. |