Unfortunately, I am quite confused about this change, so I have bunch of
questions.
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/move-optimizer.cc
File src/compiler/move-optimizer.cc (right):
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/move-optimizer.cc#newcode69
src/compiler/move-optimizer.cc:69: if (has_only_deferred) continue;
Could you explain why we skip the optimization here?
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/pipeline.cc#newcode1340
src/compiler/pipeline.cc:1340: // TODO(mtrofin): re-enable greedy once
we have bots for range preprocessing.
Do we really want to have a bot for range preprocessing? That seems to
be an overkill.
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/preprocess-live-ranges.cc
File src/compiler/preprocess-live-ranges.cc (right):
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/preprocess-live-ranges.cc#newcode83
src/compiler/preprocess-live-ranges.cc:83: int stop) {
stop -> end
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/preprocess-live-ranges.cc#newcode91
src/compiler/preprocess-live-ranges.cc:91: void
SplitRangeByClobberingDeferredBlocks(LiveRange* range,
The name is a bit confusing (it sounds like it clobbers deferred
blocks). Perhaps SplitRangeAtDeferredBlocksWithCalls?
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/preprocess-live-ranges.cc#newcode112
src/compiler/preprocess-live-ranges.cc:112: if (last_index >=
code->InstructionCount()) {
It could be more readable if you said "if (last_index >
code->LastInstructionIndex()) last_index =
code->LastInstructionIndex();". With that, you could remove the
InstructionCount method.
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/register-allocator.cc
File src/compiler/register-allocator.cc (right):
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/register-allocator.cc#newcode392
src/compiler/register-allocator.cc:392: move->AddMove(assigned,
spill_operand);
Still confused about this - where do we actually spill the deferred
block with a call? This move seems to be only inserted for ranges that
are not spilled.
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/register-allocator.h
File src/compiler/register-allocator.h (right):
https://codereview.chromium.org/1271703002/diff/60001/src/compiler/register-allocator.h#newcode456
src/compiler/register-allocator.h:456: bool
IsSpilledInSingleDeferredBlock() const {
I am really confused about this. The name refers to *single* deferred
block, but the TryCommitSpillInDeferredBlock method seems to set the
spilled_in_deferred_block_ flag even if there are multiple such blocks.
https://codereview.chromium.org/1271703002/
--
--
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.
