https://codereview.chromium.org/55933002/diff/310001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/55933002/diff/310001/src/code-stubs-hydrogen.cc#newcode712
src/code-stubs-hydrogen.cc:712: : JSArrayBuilder::DONT_FILL_WITH_HOLE;
Technically we don't need to fill this in with the hole. We could just
fill it in with 0 (memset).
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen-instructions.h#newcode1580
src/hydrogen-instructions.h:1580: virtual int RedefinedOperandIndex() {
return 0; }
Why is this needed? HForceRepresentation is explicitly eliminated after
inserting representation changes.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode1568
src/hydrogen.cc:1568: HValue* new_object = (array_length == 0)
Remove the parentheses.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode1578
src/hydrogen.cc:1578: HConstant* initial_capacity_node =
Add<HConstant>(initial_capacity);
Move this down to the case where initial-capacity-node is used.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode1790
src/hydrogen.cc:1790: if (from->ActualValue()->IsConstant() &&
to->ActualValue()->IsConstant()) {
Please put this is a separate CL (that also more widely applies
ActualValue to values you expect to be HConstant).
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode2071
src/hydrogen.cc:2071: if (is_inlined_) {
Is it good enough to use IsStub() rather than is_inlined? Then we don't
need an enum after all.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode2124
src/hydrogen.cc:2124: return const_size;
I think constant folding should take care of this. (And in other places
where you added such code, if there are such).
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode7234
src/hydrogen.cc:7234: ASSERT(!environment()->ExpressionStackIsEmpty());
I this ASSERT and the comment necessary? Doesn't the expression below
fail if ExpressionStackAt(...) is out of bounds? If it doesn't, can we
just make that fail?
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode7246
src/hydrogen.cc:7246: // to avoid creating a packed non-empty array (not
kosher).
I'd remove the (not kosher) part.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode7247
src/hydrogen.cc:7247: if (argument_count == 1 &&
!IsHoleyElementsKind(kind)) {
Can't we just do this in BuildAllocateArrayFromLength? That would seem
better encapsulated.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.cc#newcode7336
src/hydrogen.cc:7336: inline_ok =
IsHoleyElementsKind(expr->elements_kind());
Shouldn't we just deopt if the passed length is non-zero?
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.h
File src/hydrogen.h (right):
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.h#newcode1544
src/hydrogen.h:1544: bool is_inlined = false);
Please make this an enum.
https://codereview.chromium.org/55933002/diff/310001/src/hydrogen.h#newcode1586
src/hydrogen.h:1586: bool is_inlined_;
enum
https://codereview.chromium.org/55933002/diff/310001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/55933002/diff/310001/src/objects-inl.h#newcode1380
src/objects-inl.h:1380: default:
No need to add a default case if all cases are handled. Actually better
to not add a default case, so the compiler will complain when a new,
unsupported case is added to that enum.
https://codereview.chromium.org/55933002/diff/310001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/55933002/diff/310001/src/objects.h#newcode8109
src/objects.h:8109: value |= (1 << 16);
Remove the line above.
https://codereview.chromium.org/55933002/
--
--
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.
