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

Issue 10855007: Store variable counts in the flow graph builder. (Closed)

Created:
8 years, 4 months ago by Kevin Millikin (Google)
Modified:
8 years, 4 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Store variable counts in the flow graph builder. Store the constant counts of copied parameters, non-copied parameters, and (non-parameter) stack locals in the flow graph builder. This saves passing them around to flow graph member functions. Change usage of "fixed parameter" to "non-copied parameter" because it is different than what fixed parameter means in other parts of the system. R=fschneider@google.com,zerny@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=10289

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -56 lines) Patch
M runtime/vm/flow_graph_allocator.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/flow_graph_builder.h View 2 chunks +16 lines, -7 lines 0 comments Download
M runtime/vm/flow_graph_builder.cc View 15 chunks +28 lines, -48 lines 3 comments Download

Messages

Total messages: 4 (0 generated)
Kevin Millikin (Google)
This is redundant with the values in the ParsedFunction and Function, but it should be ...
8 years, 4 months ago (2012-08-06 14:00:45 UTC) #1
Kevin Millikin (Google)
http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder.cc#newcode2544 runtime/vm/flow_graph_builder.cc:2544: // TODO(fschneider): Store counts in the FlowGraphBuilder instead of ...
8 years, 4 months ago (2012-08-06 14:01:14 UTC) #2
Florian Schneider
lgtm http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder.cc#newcode2620 runtime/vm/flow_graph_builder.cc:2620: intptr_t index = comp->AsLoadLocal()->local().BitIndexIn( Not sure if it's ...
8 years, 4 months ago (2012-08-06 14:13:16 UTC) #3
zerny-google
8 years, 4 months ago (2012-08-06 14:34:13 UTC) #4
lgtm

On 2012/08/06 14:13:16, Florian Schneider wrote:
> lgtm
> 
>
http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder.cc
> File runtime/vm/flow_graph_builder.cc (right):
> 
>
http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder....
> runtime/vm/flow_graph_builder.cc:2620: intptr_t index =
> comp->AsLoadLocal()->local().BitIndexIn(
> Not sure if it's possible, but I'd break line after =
> 
>
http://codereview.chromium.org/10855007/diff/1/runtime/vm/flow_graph_builder....
> runtime/vm/flow_graph_builder.cc:2628: intptr_t index =
> comp->AsStoreLocal()->local().BitIndexIn(
> Not sure if it's possible, but I'd break line after =

Powered by Google App Engine
This is Rietveld 408576698