Unify Crankshaft Load handling into CanLoad and Load.

Unify Crankshaft Load handling into CanLoad and Load.

R=titzer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=16792

[Toon Verwaest, 2013-09-13 15:31:27]

PTAL

[titzer, 2013-09-13 15:58:12]

https://chromiumcodereview.appspot.com/24088003/diff/17001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/24088003/diff/17001/src/hydrogen.cc#ne...
src/hydrogen.cc:4565: static bool IsCompatibleForLoad(LookupResult* result,
It isn't clear to me exactly what each of the parameters mean, just based on the
name. For example, IsXXXX implies (at least to me) some singular thing is being
checked--but here we're checked a whole bunch of different criteria are
compatible.

https://chromiumcodereview.appspot.com/24088003/diff/17001/src/hydrogen.cc#ne...
src/hydrogen.cc:4606: static bool AccessFromLookup(LookupResult* lookup,
Ow, this still hurts. We're taking in three pointers and potentially updating
two of them (maybe all three?).

https://chromiumcodereview.appspot.com/24088003/diff/17001/src/hydrogen.cc#ne...
src/hydrogen.cc:4615: return LookupGetter(map, name, getter, &holder);
E.g. this little bastard still takes two pointers to handles!

[Toon Verwaest, 2013-09-17 13:59:41]

Addressed comment. PTAL again.

[titzer, 2013-09-17 15:46:28]

This is looking pretty good. Just one question...

https://chromiumcodereview.appspot.com/24088003/diff/45001/src/hydrogen.h
File src/hydrogen.h (right):

https://chromiumcodereview.appspot.com/24088003/diff/45001/src/hydrogen.h#new...
src/hydrogen.h:1981: return map_->instance_type() == JS_ARRAY_TYPE &&
I don't think we want to rely on the name here. Shouldn't we better check the
ObjectAccess instead?

[Toon Verwaest, 2013-09-17 18:24:31]

https://chromiumcodereview.appspot.com/24088003/diff/45001/src/hydrogen.h
File src/hydrogen.h (right):

https://chromiumcodereview.appspot.com/24088003/diff/45001/src/hydrogen.h#new...
src/hydrogen.h:1981: return map_->instance_type() == JS_ARRAY_TYPE &&
We didn't do a lookup because that would find native accessors. What
HObjectAccess should we check? How would we build that one? It would essentially
do the same thing: check whether the map is JS_ARRAY and whether the name of the
property is "length". It's a special property, just like string-length...

On 2013/09/17 15:46:29, titzer wrote:
> I don't think we want to rely on the name here. Shouldn't we better check the
> ObjectAccess instead?

[titzer, 2013-09-18 08:02:10]

lgtm

[Toon Verwaest, 2013-09-18 14:56:26]

Committed patchset #2 manually as r16792 (presubmit successful).