Feedback addressed, landing
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1270
src/code-stubs-hydrogen.cc:1270: explicit CodeStubGraphBuilder(Isolate*
isolate,
On 2013/11/06 10:48:12, Jakob wrote:
nit: no "explicit" necessary
Done.
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1289
src/code-stubs-hydrogen.cc:1289: HValue*
CodeStubGraphBuilder<KeyedLoadDictionaryElementStub>::
On 2013/11/06 10:48:12, Jakob wrote:
nit: I'd break after "HValue*" instead of after "::", but I leave that
up to
you.
Done.
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1302
src/code-stubs-hydrogen.cc:1302: ? hash
On 2013/11/06 10:48:12, Jakob wrote:
nit: 4 space indent
Done.
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1323
src/code-stubs-hydrogen.cc:1323: current_probe + 1);
On 2013/11/04 15:28:24, mvstanton wrote:
The recursive call is interesting here, but the base case is clear:
current_probe == kNumberDictionaryProbes, then deopt if that point is
reached.
nice!
Done.
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1326
src/code-stubs-hydrogen.cc:1326: result =
graph()->GetConstantUndefined();
On 2013/11/06 10:48:12, Jakob wrote:
As discussed, the automatic environment padding handles this, but I'd
weakly
prefer Push()ing some value (undefined, 0, take your pick) explicitly.
Done.
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1357
src/code-stubs-hydrogen.cc:1357: details_compare.End();
On 2013/11/04 15:28:24, mvstanton wrote:
nit: I would engage in the { } indentation style here like
BinaryOpStub and
FastNewClosureStub because it would be easier to parse the control
flow.
Done.
https://codereview.chromium.org/19492007/diff/160001/src/code-stubs-hydrogen.cc#newcode1370
src/code-stubs-hydrogen.cc:1370: USE(stub);
On 2013/11/04 15:28:24, mvstanton wrote:
What are the USE statements for, can they be removed?
Done.
https://codereview.chromium.org/19492007/diff/160001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/19492007/diff/160001/src/hydrogen.cc#newcode1918
src/hydrogen.cc:1918: HValue*
HGraphBuilder::BuildElementIndexHash(HValue* index) {
On 2013/11/04 15:28:24, mvstanton wrote:
Could you update the comment in macro-assembler-X.cc where it sez:
// Compute the hash code from the untagged key. This must be kept in
sync
// with ComputeIntegerHash in utils.h.
Add this location as another place to be kept in sync.
Done.
https://codereview.chromium.org/19492007/
--
--
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.
