Nice. Just some nits except can you address the comment in
HandlePolymorphicLoadNamedField()?


https://codereview.chromium.org/14966005/diff/5001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/14966005/diff/5001/src/code-stubs-hydrogen.cc#newcode431
src/code-stubs-hydrogen.cc:431: HInstruction* load =
AddInstruction(DoBuildLoadNamedField(
Since you always add the instruction, maybe the builder should just add
it? (Seems like some builders do the add, others don't.)

https://codereview.chromium.org/14966005/diff/5001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/14966005/diff/5001/src/hydrogen.cc#newcode7142
src/hydrogen.cc:7142: if (types->length() > 0 && types->length() <=
kMaxLoadPolymorphism) {
I like this if check against kMaxLoadPolymorphism, it is clearer than
before.

https://codereview.chromium.org/14966005/diff/5001/src/hydrogen.cc#newcode7185
src/hydrogen.cc:7185: instr->set_position(expr->position());
the instr->set_position() line and ast_context() line are unfortunate in
that they don't seem to have anything to do with the rest of the
function. That is okay, except that they are duplicated. Is there a way
to just run those lines once for the instruction created, whether it is
an HLoadNamedFieldPolymorphic() or a DoBuildLoadNamedField()?

Maybe you have instr = NULL; before the loop case, and then check that
value below the loop. That way you only have to write the lines I'm
complaining about once at the end of the function. You also still got to
eliminate carrying all the previous_field_offset crap out of your loop,
since you build the instruction in the loop.

https://codereview.chromium.org/14966005/diff/5001/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/14966005/diff/5001/src/hydrogen.h#newcode985
src/hydrogen.h:985: HLoadNamedField* DoBuildLoadNamedField(HValue*
object,
Maybe "Do" isn't necessary, to conform with the other Build... methods.

https://codereview.chromium.org/14966005/

--
--
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.


Reply via email to