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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months, 2 weeks ago by Vyacheslav Egorov (Google)
Modified:
11 months, 2 weeks 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) Lint Patch
M runtime/vm/code_generator.h View 1 chunk +1 line, -1 line 0 comments ? errors Download
M runtime/vm/code_generator.cc View 1 3 chunks +60 lines, -19 lines 0 comments ? errors Download
M runtime/vm/compiler.cc View 1 3 chunks +24 lines, -0 lines 0 comments ? errors Download
M runtime/vm/deopt_instructions.h View 6 chunks +44 lines, -3 lines 0 comments 0 errors Download
M runtime/vm/deopt_instructions.cc View 1 25 chunks +184 lines, -104 lines 0 comments 0 errors Download
M runtime/vm/flow_graph.h View 2 chunks +18 lines, -0 lines 0 comments 0 errors Download
M runtime/vm/flow_graph.cc View 1 chunk +2 lines, -1 line 0 comments ? errors Download
M runtime/vm/flow_graph_allocator.h View 2 chunks +4 lines, -3 lines 0 comments 0 errors Download
M runtime/vm/flow_graph_allocator.cc View 5 chunks +57 lines, -18 lines 0 comments ? errors Download
M runtime/vm/flow_graph_builder.cc View 1 chunk +5 lines, -0 lines 0 comments ? errors Download
M runtime/vm/flow_graph_compiler.h View 2 chunks +5 lines, -2 lines 0 comments ? errors Download
M runtime/vm/flow_graph_compiler.cc View 2 chunks +26 lines, -2 lines 0 comments ? errors Download
M runtime/vm/flow_graph_optimizer.h View 2 chunks +27 lines, -0 lines 0 comments ? errors Download
M runtime/vm/flow_graph_optimizer.cc View 1 14 chunks +379 lines, -40 lines 0 comments ? errors Download
M runtime/vm/il_printer.cc View 1 chunk +10 lines, -0 lines 0 comments 0 errors Download
M runtime/vm/intermediate_language.h View 2 chunks +61 lines, -1 line 0 comments ? errors Download
M runtime/vm/intermediate_language.cc View 4 chunks +23 lines, -16 lines 0 comments ? errors Download
M runtime/vm/isolate.h View 1 8 chunks +158 lines, -34 lines 0 comments 0 errors Download
M runtime/vm/isolate.cc View 3 chunks +78 lines, -0 lines 0 comments 0 errors Download
M runtime/vm/object.h View 1 2 chunks +15 lines, -3 lines 0 comments ? errors Download
M runtime/vm/object.cc View 1 chunk +9 lines, -0 lines 0 comments ? errors Download
M runtime/vm/stub_code_arm.cc View 1 chunk +7 lines, -1 line 0 comments ? errors Download
M runtime/vm/stub_code_ia32.cc View 1 chunk +10 lines, -1 line 0 comments ? errors Download
M runtime/vm/stub_code_mips.cc View 1 chunk +10 lines, -2 lines 0 comments ? errors Download
M runtime/vm/stub_code_x64.cc View 1 chunk +9 lines, -1 line 0 comments ? errors Download
A tests/language/allocation_sinking_vm_test.dart View 1 chunk +132 lines, -0 lines 0 comments ? errors Download
M tests/language/load_to_load_forwarding_test.dart View 1 chunk +27 lines, -0 lines 0 comments ? errors Download
Trybot results:
Commit:

Messages

Total messages: 8
Vyacheslav Egorov (Google)
11 months, 2 weeks ago #1
Vyacheslav Egorov (Google)
+regis & zra for ARM&MIPS changes.
11 months, 2 weeks ago #2
regis
ARM and MIPS changes LGTM
11 months, 2 weeks ago #3
zra
ARM and MIPS lgtm
11 months, 2 weeks ago #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 ...
11 months, 2 weeks ago #5
srdjan
LGTM
11 months, 2 weeks ago #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 ...
11 months, 2 weeks ago #7
Vyacheslav Egorov (Google)
11 months, 2 weeks ago #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 1280:2d3e6564b7b6