Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(712)

Issue 14935005: Implement a variation of scalar replacement for non-escaping allocations. (Closed)

Created:
7 years, 7 months ago by Vyacheslav Egorov (Google)
Modified:
7 years, 7 months ago
Reviewers:
zra, regis, srdjan
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Implement a variation of scalar replacement for non-escaping allocations. AllocationSinking pass discovers non-escaping allocations that have no input uses other than uses in the stores into its own fields. Every environment use of such allocation is replaced by a state snapshot (MaterializeObject instruction) that describes the state of each initialized field in the object. State snapshots are computed through an additional round of load-forwarding. Once snapshots are computed allocations are removed from the graph. MaterializeObject instructions are not compiled into native code but produce deoptimization instructions instead that describe how object should be materialized at deoptimization. Deoptimization instructions now follow the following format: [mat obj #1]...[mat obj #N][ret addr][... mat arguments ...][... real frames ...] - the prefix describes each object to materialize on deopt via kMaterializeObject instruction; - actual values that are needed for materialization are emited as a part of bottom-most stack frame. This is done to simplify implementation: they need to be discoverable by a GC during materialization phase. At the end of deoptimization they will be removed from the stack; - normal stack slots can refer to materialized objects via kMaterializedObjectRef instruction. Additionally this change contains fixes in load-forwarding that are needed to guarantee that all artificial LoadField instructions inserted during AllocationSinking are correctly replaced with actual values. Limitations of the current implementation: - can't eliminate allocations that flow into phis but otherwise don't actually escape; - can't sink allocations out of loops; - allocation with type arguments are not handled. R=regis@google.com, srdjan@google.com, zra@google.com Committed: https://code.google.com/p/dart/source/detail?r=22485

Patch Set 1 #

Total comments: 20

Patch Set 2 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1385 lines, -252 lines) Patch
M runtime/vm/code_generator.h View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/code_generator.cc View 1 3 chunks +60 lines, -19 lines 0 comments Download
M runtime/vm/compiler.cc View 1 3 chunks +24 lines, -0 lines 0 comments Download
M runtime/vm/deopt_instructions.h View 6 chunks +44 lines, -3 lines 0 comments Download
M runtime/vm/deopt_instructions.cc View 1 25 chunks +184 lines, -104 lines 0 comments Download
M runtime/vm/flow_graph.h View 2 chunks +18 lines, -0 lines 0 comments Download
M runtime/vm/flow_graph.cc View 1 chunk +2 lines, -1 line 0 comments Download
M runtime/vm/flow_graph_allocator.h View 2 chunks +4 lines, -3 lines 0 comments Download
M runtime/vm/flow_graph_allocator.cc View 5 chunks +57 lines, -18 lines 0 comments Download
M runtime/vm/flow_graph_builder.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M runtime/vm/flow_graph_compiler.h View 2 chunks +5 lines, -2 lines 0 comments Download
M runtime/vm/flow_graph_compiler.cc View 2 chunks +26 lines, -2 lines 0 comments Download
M runtime/vm/flow_graph_optimizer.h View 2 chunks +27 lines, -0 lines 0 comments Download
M runtime/vm/flow_graph_optimizer.cc View 1 14 chunks +379 lines, -40 lines 0 comments Download
M runtime/vm/il_printer.cc View 1 chunk +10 lines, -0 lines 0 comments Download
M runtime/vm/intermediate_language.h View 2 chunks +61 lines, -1 line 0 comments Download
M runtime/vm/intermediate_language.cc View 4 chunks +23 lines, -16 lines 0 comments Download
M runtime/vm/isolate.h View 1 8 chunks +158 lines, -34 lines 0 comments Download
M runtime/vm/isolate.cc View 3 chunks +78 lines, -0 lines 0 comments Download
M runtime/vm/object.h View 1 2 chunks +15 lines, -3 lines 0 comments Download
M runtime/vm/object.cc View 1 chunk +9 lines, -0 lines 0 comments Download
M runtime/vm/stub_code_arm.cc View 1 chunk +7 lines, -1 line 0 comments Download
M runtime/vm/stub_code_ia32.cc View 1 chunk +10 lines, -1 line 0 comments Download
M runtime/vm/stub_code_mips.cc View 1 chunk +10 lines, -2 lines 0 comments Download
M runtime/vm/stub_code_x64.cc View 1 chunk +9 lines, -1 line 0 comments Download
A tests/language/allocation_sinking_vm_test.dart View 1 chunk +132 lines, -0 lines 0 comments Download
M tests/language/load_to_load_forwarding_test.dart View 1 chunk +27 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Vyacheslav Egorov (Google)
7 years, 7 months ago (2013-05-07 21:29:00 UTC) #1
Vyacheslav Egorov (Google)
+regis & zra for ARM&MIPS changes.
7 years, 7 months ago (2013-05-07 21:48:00 UTC) #2
regis
ARM and MIPS changes LGTM
7 years, 7 months ago (2013-05-07 21:56:50 UTC) #3
zra
ARM and MIPS lgtm
7 years, 7 months ago (2013-05-07 22:16:37 UTC) #4
srdjan
https://codereview.chromium.org/14935005/diff/1/runtime/vm/code_generator.cc File runtime/vm/code_generator.cc (right): https://codereview.chromium.org/14935005/diff/1/runtime/vm/code_generator.cc#newcode1671 runtime/vm/code_generator.cc:1671: // materialization phase. Add comment what it returns. https://codereview.chromium.org/14935005/diff/1/runtime/vm/code_generator.cc#newcode1675 ...
7 years, 7 months ago (2013-05-07 23:11:45 UTC) #5
srdjan
LGTM
7 years, 7 months ago (2013-05-07 23:19:20 UTC) #6
Vyacheslav Egorov (Google)
Thanks for the review! Comments addressed. Landing. https://codereview.chromium.org/14935005/diff/1/runtime/vm/code_generator.cc File runtime/vm/code_generator.cc (right): https://codereview.chromium.org/14935005/diff/1/runtime/vm/code_generator.cc#newcode1671 runtime/vm/code_generator.cc:1671: // materialization ...
7 years, 7 months ago (2013-05-07 23:28:21 UTC) #7
Vyacheslav Egorov (Google)
7 years, 7 months ago (2013-05-07 23:45:08 UTC) #8
Message was sent while issue was closed.
Committed patchset #2 manually as r22485 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698