Chromium Code Reviews
Help | Chromium Project | Sign in
(163)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 9 months ago by Vyacheslav Egorov (Google)
Modified:
1 year, 9 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
Trybot results:
Commit:

Messages

Total messages: 8 (0 generated)
Vyacheslav Egorov (Google)
1 year, 9 months ago (2013-05-07 21:29:00 UTC) #1
Vyacheslav Egorov (Google)
+regis & zra for ARM&MIPS changes.
1 year, 9 months ago (2013-05-07 21:48:00 UTC) #2
regis
ARM and MIPS changes LGTM
1 year, 9 months ago (2013-05-07 21:56:50 UTC) #3
zra
ARM and MIPS lgtm
1 year, 9 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 ...
1 year, 9 months ago (2013-05-07 23:11:45 UTC) #5
srdjan
LGTM
1 year, 9 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 ...
1 year, 9 months ago (2013-05-07 23:28:21 UTC) #7
Vyacheslav Egorov (Google)
1 year, 9 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).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87e6a26