Mostly looks good. Some minor comments.
https://codereview.chromium.org/57123002/diff/1520001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/57123002/diff/1520001/src/code-stubs-hydrogen.cc#newcode1628
src/code-stubs-hydrogen.cc:1628: BuildReceiverCheck(receiver,
bit_field_mask);
Any reason why you don't put this outside of the index_name_split,
rather than copying it in both branches?
https://codereview.chromium.org/57123002/diff/1520001/src/code-stubs-hydrogen.cc#newcode1670
src/code-stubs-hydrogen.cc:1670: Add<HConstant>(probe * 2));
* KeyedLookupCache::kEntryLength + KeyedLookupCache::kMapIndex ?
https://codereview.chromium.org/57123002/diff/1520001/src/code-stubs-hydrogen.cc#newcode1673
src/code-stubs-hydrogen.cc:1673: Add<HConstant>(probe * 2 + 1));
+ KeyedLookupCache::kKeyIndex
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1412
src/hydrogen.cc:1412: void HGraphBuilder::BuildReceiverCheck(HValue*
receiver,
BuildCheckJSObject? You are obviously not allowing JSProxy as input, and
in the runtime JSReceiver is the superclass of JSProxy + everything
JSObject.
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1487
src/hydrogen.cc:1487: // String: check whether the String is an String
of an index. If it is,
a String of an index
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1492
src/hydrogen.cc:1492: HValue* not_array_mask =
Add<HConstant>(static_cast<int>(
I'd rather call this "not_index_mask".
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1495
src/hydrogen.cc:1495: HValue* not_array_test = AddUncasted<HBitwise>(
not_index_test
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1537
src/hydrogen.cc:1537: void
HGraphBuilder::BuildGlobalInstanceTypeCheck(HValue* receiver) {
I'd rename this to something like BuildCheckNonGlobalObject(HValue*
receiver)
I don't think you want to include the JSGlobalProxy in this list.
JSGlobalProxy internally has fast properties I believe; but it doesn't
matter, it never actually has properties. It's not a ->IsGlobalobject(),
which is only true for the JSBuiltinsObject and JSGlobalobject. (The
JSGlobalProxy in chrome has access checks turned on anyway. So you won't
even go down that path if the receiver is the JSGlobalProxy.)
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1558
src/hydrogen.cc:1558: if_global_object.ThenDeopt("global instance type
check failed");
ThenDeopt("receiver was a global object"); ?
https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1563
src/hydrogen.cc:1563: void
HGraphBuilder::BuildCheckForDictionaryProperties(
Maybe don't name this CheckXXX? Check (at least to me) indicates that
the instruction deopts.
https://codereview.chromium.org/57123002/diff/1520001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/57123002/diff/1520001/src/objects.h#newcode9082
src/objects.h:9082: kArrayIndexValueBits> {};
Don't you need a // NOLINT here? Probably a good idea to run the
presubmit checks
https://codereview.chromium.org/57123002/diff/1520001/test/mjsunit/keyed-load-dictionary-stub.js
File test/mjsunit/keyed-load-dictionary-stub.js (right):
https://codereview.chromium.org/57123002/diff/1520001/test/mjsunit/keyed-load-dictionary-stub.js#newcode14
test/mjsunit/keyed-load-dictionary-stub.js:14: return a[1];
I presume you wanted to write a[i] here?
https://codereview.chromium.org/57123002/
--
--
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/d/optout.
