Hi Dmitry, first comments. Very nice, thx,
--Michael

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);
Could you use constants for these arguments with a meaning-based name?

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8067
src/hydrogen.cc:8067: ASSERT(value->IsSmi());
The assert is unnecessary because the Smi::cast will do the same.

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8105
src/hydrogen.cc:8105: Add<HConstant>(static_cast<int32_t>(0)));
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.

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8128
src/hydrogen.cc:8128:
nit: Remove newline.

https://codereview.chromium.org/59023003/diff/110001/src/hydrogen.cc#newcode8138
src/hydrogen.cc:8138: isolate()->heap()->GetPretenureMode(),
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.

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;
How do you know these are the only cases you'll be asked about?
(kExternal, kNone), (kExternal, kExternal), (kNone, kExternal)

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