https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc
File src/compiler/register-allocator.cc (right):
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode52
src/compiler/register-allocator.cc:52: if
(block->loop_header().IsValid()) return 1;
Can't you use block->IsLoopHeader()?
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode54
src/compiler/register-allocator.cc:54: if (block->PredecessorCount() ==
0)
Style nit: only omit the parentheses if the statement fits on one line.
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode58
src/compiler/register-allocator.cc:58: sequence,
sequence->InstructionBlockAt(block->predecessors()[0]));
Can we get rid of the recursion?
More importantly, what is it supposed to compute?
At the moment, it only returns 0 or 1, that seems wrong.
Moreover, by walking up the predecessor chain you could get into a loop
and then return 1 even if the starting block was not in a loop. That
does not seem right either.
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode2593
src/compiler/register-allocator.cc:2593: auto half = ipow(b, rem);
If you wrote this function for efficiency, then you should not use
recursion here.
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode2622
src/compiler/register-allocator.cc:2622: use_count += last_use_count *
ipow(2, last_loop_count);
ipow(2, x) --> 1 << x?
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode2628
src/compiler/register-allocator.cc:2628: use_count += last_use_count *
ipow(2, last_loop_count);
ipow(2, x) --> 1 << x?
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode2845
src/compiler/register-allocator.cc:2845: // DCHECK(split_pos.IsValid());
Remove.
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode2895
src/compiler/register-allocator.cc:2895: return
LifetimePosition::Invalid();
How can this happen?
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode2931
src/compiler/register-allocator.cc:2931: if (start < pos && pos < end)
return true;
return start < pos && pos < end;
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.cc#newcode3625
src/compiler/register-allocator.cc:3625: // #include "../prints.inc"
Remove this line.
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.h
File src/compiler/register-allocator.h (right):
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.h#newcode744
src/compiler/register-allocator.h:744: };
Could we have a proper constructor that sets everything to zero? Then
reset could be simply *this = AllocatorStats().
I also like to have a separate Print method and have the operator<< call
the Print method. This makes it easier to print from the gdb. Up to you.
https://codereview.chromium.org/1132753005/diff/80001/src/compiler/register-allocator.h#newcode877
src/compiler/register-allocator.h:877: LifetimePosition
GetCorrectSplitPos(LifetimePosition pos);
Nit: ...Pos -> ...Position)
https://codereview.chromium.org/1132753005/
--
--
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.
