Issue 19370003: Enable allocation sinking for closures.

Issue 19370003: Enable allocation sinking for closures. (Closed)

Created:
7 years, 5 months ago by Florian Schneider

Modified:
7 years, 4 months ago

Reviewers:
srdjan

CC:
reviews_dartlang.org, Vyacheslav Egorov (Google), vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Enable allocation sinking for non-implicit closures. In optimized code we can eliminate the allocation of closures that are not escaping and where all calls are inlined. Closures are currently allocated with a special IL instruction (CreateClosure). This CL changes this for non-implicit closures and allocates them like normal objects. The fields for the function and the context are initialized like instance fields. This way object allocation sinking can handle closures like any other objects. R=srdjan@google.com Committed: https://code.google.com/p/dart/source/detail?r=25706

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : fixed bug in polymorphic inliner #

Total comments: 7

Patch Set 4 : addressed comments #

Created: 7 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+183 lines, -77 lines)			Patch
M	runtime/vm/flow_graph_builder.cc	View	1 2 3	4 chunks	+103 lines, -31 lines	0 comments	Download
M	runtime/vm/flow_graph_inliner.cc	View	1 2 3	5 chunks	+28 lines, -3 lines	0 comments	Download
M	runtime/vm/flow_graph_optimizer.cc	View	1 2	6 chunks	+7 lines, -9 lines	0 comments	Download
M	runtime/vm/flow_graph_type_propagator.cc	View	1 2	2 chunks	+9 lines, -3 lines	0 comments	Download
M	runtime/vm/il_printer.cc	View	1 2	2 chunks	+2 lines, -5 lines	0 comments	Download
M	runtime/vm/intermediate_language.h	View	1 2	7 chunks	+24 lines, -14 lines	0 comments	Download
M	runtime/vm/intermediate_language_arm.cc	View	1 2	1 chunk	+2 lines, -3 lines	0 comments	Download
M	runtime/vm/intermediate_language_ia32.cc	View	1 2	1 chunk	+2 lines, -3 lines	0 comments	Download
M	runtime/vm/intermediate_language_mips.cc	View	1 2	1 chunk	+2 lines, -3 lines	0 comments	Download
M	runtime/vm/intermediate_language_x64.cc	View	1 2	1 chunk	+2 lines, -3 lines	0 comments	Download
M	runtime/vm/symbols.h	View	1 2 3	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Florian Schneider

https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_inliner.cc File runtime/vm/flow_graph_inliner.cc (right): https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_inliner.cc#newcode1202 runtime/vm/flow_graph_inliner.cc:1202: callee_entry->UnuseAllInputs(); I created a separate CL with a regression ...

7 years, 4 months ago (2013-07-31 13:52:28 UTC) #3

srdjan

lgtm https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_builder.cc#newcode2026 runtime/vm/flow_graph_builder.cc:2026: String::Handle(Symbols::New(":function")), Add ":function" and ":context" to the symbols. ...

7 years, 4 months ago (2013-08-01 00:54:47 UTC) #4

Florian Schneider

Committed patchset #4 manually as r25706 (presubmit successful).

7 years, 4 months ago (2013-08-01 12:03:31 UTC) #5

Florian Schneider

7 years, 4 months ago (2013-08-05 14:41:55 UTC) #6

Message was sent while issue was closed.

https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_bui...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_bui...
runtime/vm/flow_graph_builder.cc:2026:
String::Handle(Symbols::New(":function")),
On 2013/08/01 00:54:47, srdjan wrote:
> Add ":function" and ":context" to the symbols.

Done.

https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_inl...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_inl...
runtime/vm/flow_graph_inliner.cc:686: if (alloc != NULL &&
!alloc->closure_function().IsNull()) {
On 2013/08/01 00:54:47, srdjan wrote:
> Add ()

Done.

https://codereview.chromium.org/19370003/diff/22001/runtime/vm/flow_graph_inl...
runtime/vm/flow_graph_inliner.cc:821: if (alloc != NULL &&
!alloc->closure_function().IsNull()) {
On 2013/08/01 00:54:47, srdjan wrote:
> add parentheses

Done.

Expand Messages | Collapse Messages