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;
On 2015/08/04 19:39:43, Jarin wrote:
Could you explain why we skip the optimization here?

This would pull down common fills. If the fills occur in deferred
blocks, and the closest common successor is not deferred, we lose the
optimization of spilling/filling just in deferred blocks.

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.
On 2015/08/04 19:39:43, Jarin wrote:
Do we really want to have a bot for range preprocessing? That seems to
be an
overkill.

I don't know. I know we want to have range preprocessing enabled for
linear, and I assume we'd want to go through some fuzz testing with it.
I imagined we'd do so by having a flag and flipping it on only in
certain cases - let me know how to best stage this, though.

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) {
On 2015/08/04 19:39:43, Jarin wrote:
stop -> end

Done.

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,
On 2015/08/04 19:39:43, Jarin wrote:
The name is a bit confusing (it sounds like it clobbers deferred
blocks).
Perhaps SplitRangeAtDeferredBlocksWithCalls?

Done.

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()) {
On 2015/08/04 19:39:43, Jarin wrote:
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.

Done.

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);
On 2015/08/04 19:39:43, Jarin wrote:
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.

Correct. It sets up the stage for the register allocator. The register
allocator will make the decision of where to spill, but now it will
treat the segments over deferred ranges separately from the rest of the
range.

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 {
On 2015/08/04 19:39:43, Jarin wrote:
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.

Sorry - the name was a point in time :) It's not for single at all
anymore. I'll rename.

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.

Reply via email to