https://codereview.chromium.org/11575030/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/11575030/diff/1/src/hydrogen.cc#newcode688
src/hydrogen.cc:688: }
a bit of a nit, but how about just setting exit_block to either
break_block or loop_successor and then:

if (loop_entry->IsLoopHeader()) {
  loop_entry->loop_information()->set_exit_block(exit_block);
}
return exit_block;

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.cc
File src/lithium-allocator.cc (right):

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.cc#newcode1112
src/lithium-allocator.cc:1112: HBasicBlock* block =
loop_header->loop_information()->exit_block();
How about a loop here that tries to spill to the outer-most first and
backs off to the most nested loop (which is what you currently handle?)

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.cc#newcode2159
src/lithium-allocator.cc:2159: // If the spill store was not eagerly at
parent's start then we need
nit: not eagerly emitted

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.cc#newcode2167
src/lithium-allocator.cc:2167:
(GetBlock(range->Start())->parent_loop_header() != loop_header)) {
Why don't you have to handle the case there
GetBlock(range->Start()->IsLoopHeader()) is true in a slightly different
way like you do with first_block?

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.cc#newcode2170
src/lithium-allocator.cc:2170: // reduce amount of memory writes.
You check that it's a different loop, but don't you need to check the
loop depth of loop_header and and
GetBlock(range->Start())->parent_loop_header()? You don't want to make
the situation worse by moving a spill from a loop into an inner loop?
Oh, I see. I suppose this is handled by making sure to always trying to
sink to the exit_block().

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.h
File src/lithium-allocator.h (right):

https://codereview.chromium.org/11575030/diff/1/src/lithium-allocator.h#newcode603
src/lithium-allocator.h:603: // Emit all pending spill stores for ranged
that have spilled siblings sinking
nit: ranges

https://codereview.chromium.org/11575030/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to