Benedikt, could you take another look?
I have rebased this to use the state values cache.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.cc
File src/compiler/ast-graph-builder.cc (right):
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.cc#newcode533
src/compiler/ast-graph-builder.cc:533:
graph()->SetEnd(NewNode(common()->End()));
On 2015/03/01 21:00:56, Michael Starzinger wrote:
This duplicates the end node, please don't do this.
Great catch, bad rebase. Done.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.cc#newcode537
src/compiler/ast-graph-builder.cc:537: RelaxFrameStatesWithLiveness();
On 2015/03/01 21:00:56, Michael Starzinger wrote:
Rather move this call into CreateGraph() a few lines above.
Done.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.cc#newcode548
src/compiler/ast-graph-builder.cc:548:
relaxer.Blacklist(arguments->index());
On 2015/02/27 21:09:55, titzer wrote:
Maybe "MarkPermanentlyLive(int index)" as a better name?
Done.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.cc#newcode665
src/compiler/ast-graph-builder.cc:665: void
AstGraphBuilder::Environment::MakeAllLocalsLive() {
On 2015/02/27 21:09:55, titzer wrote:
Make -> Mark?
Done.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.cc#newcode674
src/compiler/ast-graph-builder.cc:674: AstGraphBuilder::Environment*
AstGraphBuilder::Environment::Snapshot() {
On 2015/03/02 10:56:34, Michael Starzinger wrote:
nit: How about s/Snapshot/CopyAndShareLiveness/ here, or anything else
that
starts with the "Copy" prefix?
Done.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.h
File src/compiler/ast-graph-builder.h (right):
https://codereview.chromium.org/949743002/diff/130001/src/compiler/ast-graph-builder.h#newcode359
src/compiler/ast-graph-builder.h:359: // shifted by 1 (receiver is
parameter index -1 but environment index 0).
On 2015/03/02 10:56:34, Michael Starzinger wrote:
nit: I think the second sentence in the comment makes more sense for
the
implementation, not the interface. Can we move it to where the magic
off-by one
calculation is performed?
Done.
https://codereview.chromium.org/949743002/diff/130001/src/compiler/liveness-analyzer.h
File src/compiler/liveness-analyzer.h (right):
https://codereview.chromium.org/949743002/diff/130001/src/compiler/liveness-analyzer.h#newcode21
src/compiler/liveness-analyzer.h:21: void RelaxFrameState(Node*
frame_state, ZoneVector<bool>* liveness);
On 2015/02/27 21:03:54, titzer wrote:
Can we use a BitVector here and in the implementation. The space usage
of
ZoneVector<bool> is probably not a big issue, but with BitVector it
will
definitely be smaller, and we are already using BitVector in other
analyses like
loop assignment analsysis.
Done. (Will BitVector be really smaller? Why?)
https://codereview.chromium.org/949743002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
