Addressed first set of comments.
https://codereview.chromium.org/14146005/diff/33001/src/flag-definitions.h
File src/flag-definitions.h (right):
https://codereview.chromium.org/14146005/diff/33001/src/flag-definitions.h#newcode194
src/flag-definitions.h:194: DEFINE_bool(track_double_fields, true,
"track fields with double values")
On 2013/04/24 15:23:00, danno wrote:
These should be off by default and enabled in another CL
Done.
https://codereview.chromium.org/14146005/diff/33001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/14146005/diff/33001/src/hydrogen-instructions.h#newcode5550
src/hydrogen-instructions.h:5550: // if (FLAG_track_double_fields &&
index == 1 &&
On 2013/04/24 15:23:00, danno wrote:
Remove commented code?
Done.
https://codereview.chromium.org/14146005/diff/33001/src/hydrogen-instructions.h#newcode5578
src/hydrogen-instructions.h:5578: // (!FLAG_track_double_fields ||
field_representation_ != DOUBLE) &&
On 2013/04/24 15:23:00, danno wrote:
Remove commented code.
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://codereview.chromium.org/14146005/diff/33001/src/ia32/lithium-codegen-ia32.cc#newcode2973
src/ia32/lithium-codegen-ia32.cc:2973: XMMRegister xmm =
ToDoubleRegister(instr->result());
On 2013/04/24 15:23:00, danno wrote:
rename this to "result"
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ia32/lithium-codegen-ia32.cc#newcode2983
src/ia32/lithium-codegen-ia32.cc:2983: // TODO(verwaest): Implement
noSSE2 support.
On 2013/04/24 15:23:00, danno wrote:
Use LoadKeyed as an example how to do this.
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ia32/lithium-codegen-ia32.cc#newcode4265
src/ia32/lithium-codegen-ia32.cc:4265: if
(instr->value()->IsConstantOperand()) {
On 2013/04/24 15:23:00, danno wrote:
If it's a double, disable constant in lithium-ia32.cc
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ic.cc
File src/ic.cc (right):
https://codereview.chromium.org/14146005/diff/33001/src/ic.cc#newcode185
src/ic.cc:185: if (target->type() != Code::NORMAL) {
On 2013/04/24 15:23:00, danno wrote:
Can you please add a comment here why this compare is relevant?
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ic.cc#newcode517
src/ic.cc:517: if (receiver->map()->is_invalid_transition()) {
On 2013/04/24 15:23:00, danno wrote:
is_invalid_transition_target or is_invalid. is_deprecated?
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ic.cc#newcode1106
src/ic.cc:1106: MapHandleList maps;
On 2013/04/24 15:23:00, danno wrote:
Comment that this is special handling that is only needed until we
have
polymorphic store ICs.
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ic.cc#newcode1214
src/ic.cc:1214:
On 2013/04/24 15:23:00, danno wrote:
extraneous whitespace change
Done.
https://codereview.chromium.org/14146005/diff/33001/src/ic.cc#newcode1508
src/ic.cc:1508: return lookup->CanStore(value);
On 2013/04/24 15:23:00, danno wrote:
CanHoldValue?
Done.
https://codereview.chromium.org/14146005/diff/33001/src/json-parser.h
File src/json-parser.h (right):
https://codereview.chromium.org/14146005/diff/33001/src/json-parser.h#newcode431
src/json-parser.h:431: map, field, value->RequiredRepresentation());
On 2013/04/24 15:23:00, danno wrote:
OptimalRepresentation
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/14146005/diff/33001/src/objects-inl.h#newcode61
src/objects-inl.h:61: int value = value_ << 1;
On 2013/04/24 15:23:00, danno wrote:
Comment why this is needed?
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects-inl.h#newcode3563
src/objects-inl.h:3563: bool Map::CanTransitionBeInvalidated() {
On 2013/04/24 15:23:00, danno wrote:
CanBeDeprecated
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/14146005/diff/33001/src/objects.cc#newcode2172
src/objects.cc:2172: // If fields were added (or removed), rewrite the
instance.
On 2013/04/24 16:08:20, danno wrote:
ASSERT(target_number_of_fileds >= NumberOfFields())
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects.cc#newcode2294
src/objects.cc:2294: MaybeObject* Map::CopyGeneralizeRepresentation(
On 2013/04/24 16:08:20, danno wrote:
CopyGeneralizeAllRepresentations
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects.cc#newcode2478
src/objects.cc:2478: int descriptor =
split_map->NumberOfOwnDescriptors();
On 2013/04/24 16:08:20, danno wrote:
split_descriptors
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects.cc#newcode2488
src/objects.cc:2488: for (; descriptor < descriptors; descriptor++) {
On 2013/04/24 16:08:20, danno wrote:
maybe a separate loop variable?
Done.
https://codereview.chromium.org/14146005/diff/33001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/14146005/diff/33001/src/objects.h#newcode1069
src/objects.h:1069: inline bool FitsRepresentation(Representation
representation) {
On 2013/04/24 15:23:00, danno wrote:
Check for FLAG_track_double_fields
Done.
https://codereview.chromium.org/14146005/
--
--
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.
