Comments addressed + take a look at a change in lithium-codegen-x64.cc
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(
On 2013/11/20 13:50:50, danno wrote:
nit: remove this whitespace change.
Done.
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,
On 2013/11/20 13:50:50, danno wrote:
this should be called "builder". Consider moving this method to the
HGraphBuilder super class, that avoids having to use graph->
everywhere.
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8067
src/hydrogen.cc:8067: offset < ViewClass::kSizeWithInternalFields;
On 2013/11/20 13:50:50, danno wrote:
nit: alignment
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8087
src/hydrogen.cc:8087: JSArrayBuffer::kWeakFirstViewOffset)));
On 2013/11/20 13:50:50, danno wrote:
nit: 4 char indent. Perhaps move the object access into a local.
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8136
src/hydrogen.cc:8136:
static_cast<Literal*>(arguments->at(kArrayIdArg))->value();
On 2013/11/20 13:50:50, danno wrote:
4-char indent
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8197
src/hydrogen.cc:8197: HObjectAccess::ForJSObjectOffset(
On 2013/11/20 13:50:50, danno wrote:
nit: 4 char indent
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8198
src/hydrogen.cc:8198: JSArrayBuffer::kBackingStoreOffset,
Representation::External()));
On 2013/11/20 13:50:50, danno wrote:
Same here. Maybe move the access into a local variable? Makes the line
shorter.
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8206
src/hydrogen.cc:8206: zone(), graph()->GetInvalidContext(),
backing_store, byte_offset);
On 2013/11/20 13:50:50, danno wrote:
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.
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8215
src/hydrogen.cc:8215: ExternalArray::kExternalPointerOffset,
Representation::External()),
On 2013/11/20 13:50:50, danno wrote:
nit: 4 char indent?
Done.
https://codereview.chromium.org/59023003/diff/380001/src/hydrogen.cc#newcode8235
src/hydrogen.cc:8235: byte_offset_smi.End();
On 2013/11/20 13:50:50, danno wrote:
Shouldn't this be after the } below? It should always be called.
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.
