https://codereview.chromium.org/59023003/diff/380001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen-instructions.h#newcode4751
src/hydrogen-instructions.h:4751: static HInstruction* New(
nit: remove this whitespace change.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8060
src/hydrogen.cc:8060: HGraphBuilder* graph,
this should be called "builder". Consider moving this method to the
HGraphBuilder super class, that avoids having to use graph-> everywhere.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8067
src/hydrogen.cc:8067: offset < ViewClass::kSizeWithInternalFields;
nit: alignment
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8087
src/hydrogen.cc:8087: JSArrayBuffer::kWeakFirstViewOffset)));
nit: 4 char indent. Perhaps move the object access into a local.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8136
src/hydrogen.cc:8136:
static_cast<Literal*>(arguments->at(kArrayIdArg))->value();
4-char indent
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8197
src/hydrogen.cc:8197: HObjectAccess::ForJSObjectOffset(
nit: 4 char indent
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8198
src/hydrogen.cc:8198: JSArrayBuffer::kBackingStoreOffset,
Representation::External()));
Same here. Maybe move the access into a local variable? Makes the line
shorter.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8206
src/hydrogen.cc:8206: zone(), graph()->GetInvalidContext(),
backing_store, byte_offset);
You should be able to use Add<> now, shouldn't you? Add returns the
typed instruction pointer, so you can call ClearFlag after-the-fact.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8215
src/hydrogen.cc:8215: ExternalArray::kExternalPointerOffset,
Representation::External()),
nit: 4 char indent?
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8235
src/hydrogen.cc:8235: byte_offset_smi.End();
Shouldn't this be after the } below? It should always be called.
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.
