Comments addressed, PTAL
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#newcode8065
src/hydrogen.cc:8065: ASSERT(arguments->at(1)->node_type() ==
AstNode::kLiteral);
On 2013/11/18 10:44:49, mvstanton wrote:
Could you use constants for these arguments with a meaning-based name?
Done. Great suggestion, because it uncovered a bug :)
https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8067
src/hydrogen.cc:8067: ASSERT(value->IsSmi());
On 2013/11/18 10:44:49, mvstanton wrote:
The assert is unnecessary because the Smi::cast will do the same.
I am paranoid lately.
https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8105
src/hydrogen.cc:8105: Add<HConstant>(static_cast<int32_t>(0)));
On 2013/11/18 10:44:49, mvstanton wrote:
Not something we have to do now, but it would be neat if we had a
memset
instruction that could do this very efficiently. I can think of other
places
we'd use it.
Agreed.
https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8128
src/hydrogen.cc:8128:
On 2013/11/18 10:44:49, mvstanton wrote:
nit: Remove newline.
Done.
https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8138
src/hydrogen.cc:8138: isolate()->heap()->GetPretenureMode(),
On 2013/11/18 10:44:49, mvstanton wrote:
It would be good if you continue passing NOT_TENURED here, like the
current
runtime call does. The reason is that for the allocation-site-based
pretenuring
work, we've identified sites that make use of this global mode and are
(rather
painfully) addressing them in turn. I don't want that set to grow
larger for the
moment.
Done.
https://codereview.chromium.org/59023003/diff/110001/src/property-details.h
File src/property-details.h (right):
https://codereview.chromium.org/59023003/diff/110001/src/property-details.h#newcode136
src/property-details.h:136: if (kind_ == kNone && other.kind_ ==
kExternal) return false;
On 2013/11/18 10:44:49, mvstanton wrote:
How do you know these are the only cases you'll be asked about?
(kExternal,
kNone), (kExternal, kExternal), (kNone, kExternal)
Representation is a lattice where kNone is the bottom element and
kExternal is uncomparable with anything else. We assert on uncomparable.
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.
