Some very-surface-level comments.
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc
File src/compiler/register-allocator.cc (right):
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode2101
src/compiler/register-allocator.cc:2101: range_count++;
How is range_count used? Seems to be a local variable that is only
incremented. Was it previously part of debug-only traces?
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode2715
src/compiler/register-allocator.cc:2715: // is
reflow block of comments?
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode2751
src/compiler/register-allocator.cc:2751: // still
reflow comment
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode2770
src/compiler/register-allocator.cc:2770: // by
reflow comments
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode2792
src/compiler/register-allocator.cc:2792: // contained
can put "ranges." on the same line
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode2808
src/compiler/register-allocator.cc:2808: bool empty() { return
storage_.empty(); }
const?
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode3081
src/compiler/register-allocator.cc:3081: CHECK(!range->IsFixed());
Just checking -- DCHECK or CHECK ?
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode3282
src/compiler/register-allocator.cc:3282: CHECK_EQ(0, queue_.size());
Do you need CHECK or DCHECK is okay? Similar below.
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.cc#newcode3947
src/compiler/register-allocator.cc:3947: moves_counter++;
Similar to the range_count earlier. This local variable seems to only
get incremented but not observed.
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h
File src/compiler/register-allocator.h (right):
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h#newcode542
src/compiler/register-allocator.h:542: bool IsSlotAssigned() { return
assigned_slot_ != kUnassignedSlot; }
const
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h#newcode925
src/compiler/register-allocator.h:925: typedef
ZoneSet<AllocatedInterval, AllocatedInterval::Comparer>::const_iterator
"ZoneSet<AllocatedInterval, AllocatedInterval::Comparer>" also shows up
a couple times, so if it would help you could also typedef that?
If you want to typedef, may want to narrow its scope to the
conflict_iterator though since it's only referenced in the private
parts.
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h#newcode956
src/compiler/register-allocator.h:956: conflict_iterator(UseInterval* q,
Is this ctor variant used? Not knowing much it seemed a bit odd to take
q, s parameters but then ignore them and not assign them to the query_
and storage_ fields.
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h#newcode1006
src/compiler/register-allocator.h:1006: bool IsGroup() { return is_grp_;
}
const
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h#newcode1015
src/compiler/register-allocator.h:1015:
can remove some an extra newline here and line 1020/1021
https://codereview.chromium.org/1157663007/diff/40001/src/compiler/register-allocator.h#newcode1016
src/compiler/register-allocator.h:1016: LiveRange* get_range() {
const
https://codereview.chromium.org/1157663007/
--
--
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.
