PTAL. Comments addressed, with reservations :)
https://codereview.chromium.org/59023003/diff/510001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/59023003/diff/510001/src/hydrogen.cc#newcode8176
src/hydrogen.cc:8176:
HObjectAccess::ForJSObjectOffset(JSTypedArray::kLengthOffset),
On 2013/11/21 20:44:35, danno wrote:
Here an elsewhere, does it perhaps make sense to wrap these
HObjectAccess
builders in utility functions? That's what we've done elsewhere, it
provides
another level of indirection, e.g.:
HObjectAccess::ForJSTypedArrayLength()
Done, albeit reluctantly. I see the value of encapsulating field
information (esp. representations), however I am not too happy to see
one huge sheet of all fields of all objects in HObjectAccess class.
https://codereview.chromium.org/59023003/diff/510001/src/hydrogen.cc#newcode8180
src/hydrogen.cc:8180: Add<HAllocate>(
On 2013/11/21 20:44:35, danno wrote:
nit: 4 char indent
Done.
https://codereview.chromium.org/59023003/diff/510001/src/hydrogen.cc#newcode8188
src/hydrogen.cc:8188:
On 2013/11/21 20:44:35, danno wrote:
nit: Group the load/store pairs and their preparation that belong
together, e.g.
remove this line
Done.
https://codereview.chromium.org/59023003/diff/510001/src/hydrogen.cc#newcode8191
src/hydrogen.cc:8191: Add<HConstant>(external_array_map));
On 2013/11/21 20:44:35, danno wrote:
nit: And insert one after this line
Done.
https://codereview.chromium.org/59023003/diff/510001/src/hydrogen.cc#newcode8195
src/hydrogen.cc:8195:
On 2013/11/21 20:44:35, danno wrote:
nit: remove this line
Done.
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.
