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

Issue 145133009: Improve aliasing info for load elimination of array loads. (Closed)

Created:
6 years, 11 months ago by Florian Schneider
Modified:
6 years, 11 months ago
Reviewers:
Cutch
CC:
reviews_dartlang.org, vm-dev_dartlang.org, Vyacheslav Egorov (Google)
Visibility:
Public.

Description

Improve aliasing info for load elimination of array loads. The optimizer can now track side effects to array locations at a constant index when eliminating array loads. Stores to arrays with a constant index affect loads from arrays at the same constant index _and_ loads from an unknown (non-constant) index. Stores to an array with an unknown index affect all array loads. For instance code like var a = new List(2); a[0] = 123; a[1] = 456; print(a[0] + a[1]); we can now eliminate both loads from a[0] and a[1], where before only the loads from a[1] was eliminated. I changed the internal encoding of Alias to use BitField instead of plain integers. This makes it a little easire to extend. R=johnmccutchan@google.com Committed: https://code.google.com/p/dart/source/detail?r=32014

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+179 lines, -21 lines) Patch
M runtime/vm/flow_graph_optimizer.cc View 11 chunks +134 lines, -21 lines 6 comments Download
M tests/language/vm/load_to_load_forwarding_vm_test.dart View 2 chunks +45 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Florian Schneider
https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimizer.cc File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimizer.cc#newcode301 runtime/vm/flow_graph_optimizer.cc:301: flow_graph()->GetConstant(Smi::Handle(Smi::New(ix))); Unrelated cleanup to use constant pool here. https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimizer.cc#newcode4706 ...
6 years, 11 months ago (2014-01-23 13:02:29 UTC) #1
Cutch
LGTM with some suggestions. https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimizer.cc File runtime/vm/flow_graph_optimizer.cc (right): https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimizer.cc#newcode5121 runtime/vm/flow_graph_optimizer.cc:5121: // Non-constant indexes alias all ...
6 years, 11 months ago (2014-01-24 23:07:15 UTC) #2
Florian Schneider
Committed patchset #1 manually as r32014 (presubmit successful).
6 years, 11 months ago (2014-01-27 11:06:56 UTC) #3
Florian Schneider
6 years, 11 months ago (2014-01-27 14:29:00 UTC) #4
Message was sent while issue was closed.
https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimi...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:5121: // Non-constant indexes alias all
constant indexes.
On 2014/01/24 23:07:15, Cutch wrote:
> Can these two loops not be merged? 

No, at first I thought they can, but Slava pointed out that the ordering matters
here.

https://codereview.chromium.org/145133009/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:5327: typedef IndexIdPair Pair;
On 2014/01/24 23:07:15, Cutch wrote:
> Why not just pick one name for this class: Pair or IndexIdPair.

Done. I can replace Pair below with IndexIdPair, but still need the typedef
because the hash map implementation requires the type parameter to have a type
Pair defined.

Powered by Google App Engine
This is Rietveld 408576698