https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-gvn.cc
File src/hydrogen-gvn.cc (right):
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-gvn.cc#newcode779
src/hydrogen-gvn.cc:779: for (HInstructionIterator it(block);
!it.Done(); it.Advance()) {
On 2013/07/08 12:44:06, Michael Starzinger wrote:
Switching to the HInstructionIterator will break for cases where the
instruction
is unlinked from the graph. See the following comment about that.
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-gvn.cc#newcode799
src/hydrogen-gvn.cc:799: if (instr->IsLinked() && !flags.IsEmpty()) {
On 2013/07/08 12:44:06, Michael Starzinger wrote:
Instead of doing the IsLinked() checks here, could we just to a
"continue" after
the side-effect dominator notification? Of course that require us to
make
HInstructionIterator resilient against concurrent modification (i.e.
remember
the "next_" field in the iterator).
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-gvn.cc#newcode807
src/hydrogen-gvn.cc:807: if (instr->IsLinked() &&
instr->CheckFlag(HValue::kUseGVN)) {
On 2013/07/08 12:44:06, Michael Starzinger wrote:
Likewise.
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.cc#newcode1658
src/hydrogen-instructions.cc:1658: HValue* dominator) {
On 2013/07/08 12:44:06, Michael Starzinger wrote:
nit: Indentation if off.
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.cc#newcode3206
src/hydrogen-instructions.cc:3206: HValue* dominator) {
On 2013/07/08 12:44:06, Michael Starzinger wrote:
nit: Indentation is off.
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.cc#newcode3208
src/hydrogen-instructions.cc:3208: if (FLAG_use_allocation_folding) {
On 2013/07/08 12:40:14, titzer wrote:
if (!FLAG_...) return;
if (!...) return;
Can remove two levels of nesting here.
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.cc#newcode3230
src/hydrogen-instructions.cc:3230: HInstruction* dominator_instr =
HInstruction::cast(dominator);
On 2013/07/08 12:40:14, titzer wrote:
You already have dominator as dominator_allocate_instr.
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.cc#newcode3240
src/hydrogen-instructions.cc:3240: HConstant* filler_map =
block->graph()->GetConstantFreeSpaceMap();
On 2013/07/08 12:40:14, titzer wrote:
Don't make this a global constant in the graph. That increases its
live range,
thus register pressure and spilling. It's fine to create this constant
at each
allocation site, IMO. Maybe if you have multiple folded allocations
you can
insert this constant after the intial HAllocate and reuse the same
one, but
making it global is probably not worth it.
OK.
I am leaving this code since it will disappear anyway after fixing the
new space iterator.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.h#newcode5811
src/hydrogen-instructions.h:5811: if (no_observable_side_effects) {
On 2013/07/08 12:44:06, Michael Starzinger wrote:
Can we add a TODO here that this argument should go away once we no
longer need
to do initialization of folded allocation segments?
Done.
https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.h#newcode5811
src/hydrogen-instructions.h:5811: if (no_observable_side_effects) {
On 2013/07/08 12:40:14, titzer wrote:
Please don't add more flags to this constructor; you should probably
set these
flags in your special insertion routine. I'm not really sure why you
are doing
this anyway...since I don't think the instructions are GVN'd after
that....
This is needed otherwise the HGraph Verifier is not happy. But I will
add a TODO as Michael suggested.
https://codereview.chromium.org/18596005/
--
--
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/groups/opt_out.
