LGTM if comments are addressed. I like it!
https://codereview.chromium.org/951553005/diff/80001/src/compiler/instruction-selector.h
File src/compiler/instruction-selector.h (right):
https://codereview.chromium.org/951553005/diff/80001/src/compiler/instruction-selector.h#newcode24
src/compiler/instruction-selector.h:24: class BasicBlock;
nit: Please alpha-sort.
https://codereview.chromium.org/951553005/diff/80001/src/compiler/instruction.h
File src/compiler/instruction.h (right):
https://codereview.chromium.org/951553005/diff/80001/src/compiler/instruction.h#newcode695
src/compiler/instruction.h:695: // TODO(dcarney): remove this class and
just use an uint32_t.
suggestion: Not sure about this TODO, better ask Benedikt about his
opinion.
https://codereview.chromium.org/951553005/diff/80001/src/compiler/jump-threading.cc
File src/compiler/jump-threading.cc (right):
https://codereview.chromium.org/951553005/diff/80001/src/compiler/jump-threading.cc#newcode12
src/compiler/jump-threading.cc:12: typedef RpoNumber RpoNumber;
nit: Redundant typedef is redundant.
https://codereview.chromium.org/951553005/diff/80001/src/compiler/schedule.cc
File src/compiler/schedule.cc (right):
https://codereview.chromium.org/951553005/diff/80001/src/compiler/schedule.cc#newcode316
src/compiler/schedule.cc:316: for (BasicBlock* block : *s.rpo_order()) {
Since this printer already iterates the rpo_order vector, it can only be
applied on a sealed schedule. Hence we can also print "B{rpo_number}"
here as well.
https://codereview.chromium.org/951553005/diff/80001/test/cctest/compiler/test-instruction.cc
File test/cctest/compiler/test-instruction.cc (left):
https://codereview.chromium.org/951553005/diff/80001/test/cctest/compiler/test-instruction.cc#oldcode134
test/cctest/compiler/test-instruction.cc:134:
CHECK_EQ(block->id().ToInt(), R.BlockAt(block)->id().ToInt());
Can we preserve this check? It should still hold, right?
https://codereview.chromium.org/951553005/diff/80001/test/cctest/compiler/test-jump-threading.cc
File test/cctest/compiler/test-jump-threading.cc (right):
https://codereview.chromium.org/951553005/diff/80001/test/cctest/compiler/test-jump-threading.cc#newcode16
test/cctest/compiler/test-jump-threading.cc:16: typedef RpoNumber
RpoNumber;
nit: Redundant typedef is redundant.
https://codereview.chromium.org/951553005/
--
--
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.
