Feedback addressed, please take another look.

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#newcode1670
src/code-stubs-hydrogen.cc:1670: Add<HConstant>(probe * 2));
On 2014/06/10 11:33:43, Toon Verwaest wrote:
* KeyedLookupCache::kEntryLength + KeyedLookupCache::kMapIndex ?

Done.

https://codereview.chromium.org/57123002/diff/1520001/src/code-stubs-hydrogen.cc#newcode1673
src/code-stubs-hydrogen.cc:1673: Add<HConstant>(probe * 2 + 1));
On 2014/06/10 11:33:43, Toon Verwaest wrote:
+ KeyedLookupCache::kKeyIndex

Done.

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,
On 2014/06/10 11:33:43, Toon Verwaest wrote:
BuildCheckJSObject? You are obviously not allowing JSProxy as input,
and in the
runtime JSReceiver is the superclass of JSProxy + everything JSObject.

Done.

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,
On 2014/06/10 11:33:43, Toon Verwaest wrote:
a String of an index

Done.

https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1492
src/hydrogen.cc:1492: HValue* not_array_mask =
Add<HConstant>(static_cast<int>(
On 2014/06/10 11:33:43, Toon Verwaest wrote:
I'd rather call this "not_index_mask".

Done.

https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1495
src/hydrogen.cc:1495: HValue* not_array_test = AddUncasted<HBitwise>(
On 2014/06/10 11:33:43, Toon Verwaest wrote:
not_index_test

Done.

https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1537
src/hydrogen.cc:1537: void
HGraphBuilder::BuildGlobalInstanceTypeCheck(HValue* receiver) {
On 2014/06/10 11:33:44, Toon Verwaest wrote:
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.)

Done.

https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1558
src/hydrogen.cc:1558: if_global_object.ThenDeopt("global instance type
check failed");
On 2014/06/10 11:33:43, Toon Verwaest wrote:
ThenDeopt("receiver was a global object"); ?

Done.

https://codereview.chromium.org/57123002/diff/1520001/src/hydrogen.cc#newcode1563
src/hydrogen.cc:1563: void
HGraphBuilder::BuildCheckForDictionaryProperties(
On 2014/06/10 11:33:43, Toon Verwaest wrote:
Maybe don't name this CheckXXX? Check (at least to me) indicates that
the
instruction deopts.

Done.

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> {};
On 2014/06/10 11:33:44, Toon Verwaest wrote:
Don't you need a // NOLINT here? Probably a good idea to run the
presubmit
checks

Done.

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];
On 2014/06/10 11:33:44, Toon Verwaest wrote:
I presume you wanted to write a[i] here?

Done.

https://codereview.chromium.org/57123002/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to