Looking good with some comments.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/instruction.h
File src/compiler/instruction.h (right):
https://codereview.chromium.org/1305393003/diff/1/src/compiler/instruction.h#newcode784
src/compiler/instruction.h:784:
Nit: please add operator!= for consistency.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.cc
File src/compiler/live-range-separator.cc (right):
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.cc#newcode38
src/compiler/live-range-separator.cc:38: return
RpoNumber::FromInt(code->InstructionBlockCount() - 1);
I guess that's always last then? How about
for(...) {
...
if (!block_at_i->IsDeferred()) break;
last = at_i;
}
return last;
instead?
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.cc#newcode44
src/compiler/live-range-separator.cc:44: for (int blk_id = 0; blk_id <
code->InstructionBlockCount(); ++blk_id) {
This looks like O(n*2) in the worst case. I guess that's not an issue,
but it would be nice if you could add a comment to this loop, so
everyone is aware.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h
File src/compiler/live-range-separator.h (right):
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode5
src/compiler/live-range-separator.h:5: #ifndef
V8_PREPROCESS_LIVE_RANGES_H_
Nit: Rename header guard.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode8
src/compiler/live-range-separator.h:8: #include
"src/compiler/register-allocator.h"
Don't include the header here, but forward declare Zone and
RegisterAllocationData below.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode24
src/compiler/live-range-separator.h:24: RegisterAllocationData* data() {
return data_; }
Nit: mark these accessors const.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode27
src/compiler/live-range-separator.h:27: RegisterAllocationData* data_;
Nit: use const fields, i.e. RegisterAllocationData* const data_;
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode40
src/compiler/live-range-separator.h:40: RegisterAllocationData* data() {
return data_; }
See above.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode43
src/compiler/live-range-separator.h:43: RegisterAllocationData* data_;
See above.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/register-allocator.cc
File src/compiler/register-allocator.cc (right):
https://codereview.chromium.org/1305393003/diff/1/src/compiler/register-allocator.cc#newcode720
src/compiler/register-allocator.cc:720: DCHECK(splinter_parent->Start()
< Start() && !splinter_parent->IsChild());
Use separate DCHECKs here instead of &&.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/register-allocator.cc#newcode2793
src/compiler/register-allocator.cc:2793: auto range = *i;
Nit: Use LiveRange* instead of auto
https://codereview.chromium.org/1305393003/diff/1/src/compiler/register-allocator.cc#newcode2798
src/compiler/register-allocator.cc:2798: auto other = *j;
Same here.
https://codereview.chromium.org/1305393003/
--
--
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.
