addressed comments, ptal again
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(
It seems more in sync with the other builds to not add it; so I'll leave
it like it was.
On 2013/05/07 10:17:17, mvstanton wrote:
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) {
On 2013/05/07 10:17:17, mvstanton wrote:
I like this if check against kMaxLoadPolymorphism, it is clearer than
before.
Done.
https://codereview.chromium.org/14966005/diff/5001/src/hydrogen.cc#newcode7185
src/hydrogen.cc:7185: instr->set_position(expr->position());
On 2013/05/07 10:17:17, mvstanton wrote:
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.
Done.
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,
Unfortunately it's necessary because you can't overload methods between
2 different classes.
On 2013/05/07 10:17:17, mvstanton wrote:
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.
