https://codereview.chromium.org/59023003/diff/110001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen-instructions.h#newcode4627
src/hydrogen-instructions.h:4627: static HAdd* NewExternalPointerOffset(
This doesn't really work well the the Add<> templates. I think better
would be a 5th parameter to new of type Representation that is by
default None(), but when it's something else it sets the representation
of the LHS and the observed output representation. That way, it's still
possible to use the Add<> templates with an additional parameter.

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8161
src/hydrogen.cc:8161: external_pointer->ClearFlag(HValue::kCanOverflow);
This should be folded into the constructor of the Add based on the
representation, probably.

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8176
src/hydrogen.cc:8176: byte_offset_smi.Else();
Perhaps use the {} indenting style used in code-stubs-hydrogen to help
parse the IfBuilder control flow?

https://codereview.chromium.org/59023003/diff/180001/src/property-details.h
File src/property-details.h (right):

https://codereview.chromium.org/59023003/diff/180001/src/property-details.h#newcode134
src/property-details.h:134: if (kind_ == kExternal && other.kind_ ==
kNone) return true;
Can you please update the test-representation.cc test case to fully
cover your changes (and perhaps kExternal in general)?

https://codereview.chromium.org/59023003/

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