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:
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Nit: please add operator!= for consistency.
Done.
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);
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
I guess that's always last then? How about
for(...) {
...
if (!block_at_i->IsDeferred()) break;
last = at_i;
}
return last;
instead?
Done.
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) {
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
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.
I fixed it to be O(n), by skipping over last + 1, which we know to be
not deferred.
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_
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Nit: Rename header guard.
Done.
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"
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Don't include the header here, but forward declare Zone and
RegisterAllocationData below.
Done.
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_; }
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Nit: mark these accessors const.
Done.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode27
src/compiler/live-range-separator.h:27: RegisterAllocationData* data_;
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Nit: use const fields, i.e. RegisterAllocationData* const data_;
Done.
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_; }
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
See above.
Done.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/live-range-separator.h#newcode43
src/compiler/live-range-separator.h:43: RegisterAllocationData* data_;
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
See above.
Done.
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());
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Use separate DCHECKs here instead of &&.
Done.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/register-allocator.cc#newcode2793
src/compiler/register-allocator.cc:2793: auto range = *i;
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Nit: Use LiveRange* instead of auto
Done.
https://codereview.chromium.org/1305393003/diff/1/src/compiler/register-allocator.cc#newcode2798
src/compiler/register-allocator.cc:2798: auto other = *j;
On 2015/08/24 04:33:26, Benedikt Meurer wrote:
Same here.
Done.
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.
