FastNewClosureStub failed to load the cached literals array.

FastNewClosureStub failed to load the cached literals array.

Also, less efficient code was generated because of negative keyed load
offsets. I changed this to benefit from HLoadKeyed dehoisting.

BUG=v8:3185
LOG=N
R=yangguo@chromium.org

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

[mvstanton, 2014-02-28 16:15:30]

Hi Yang, PTAL.
--Michael

[Yang, 2014-03-03 08:27:02]

https://codereview.chromium.org/184103004/diff/20001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/184103004/diff/20001/src/code-stubs-hydrogen....
src/code-stubs-hydrogen.cc:1272: // slot iterator is pointing behind the record
we want to examine.
This comment seems not to match the code...
if the slot iterator is indeed pointing behind the record we want to examine,
wouldn't we need to use negative indices to load fields? The for-loop in the
comment above suggests that we start at the last record, but then the
loop_builder is using kPostDecrement...

[Yang, 2014-03-03 08:48:02]

lgtm if comment removed.

[mvstanton, 2014-03-03 09:35:51]

Thanks Yang, addressed comments. Will check in after tree opens.

https://codereview.chromium.org/184103004/diff/20001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/184103004/diff/20001/src/code-stubs-hydrogen....
src/code-stubs-hydrogen.cc:1272: // slot iterator is pointing behind the record
we want to examine.
On 2014/03/03 08:27:02, Yang wrote:
> This comment seems not to match the code...
> if the slot iterator is indeed pointing behind the record we want to examine,
> wouldn't we need to use negative indices to load fields? The for-loop in the
> comment above suggests that we start at the last record, but then the
> loop_builder is using kPostDecrement...

Right, the comment was old. I changed the loop iterator to point at the *start*
of each record we want to examine. Removing!

[mvstanton, 2014-03-10 09:11:27]

Hi Yang,
There was a logic error in my refactoring. I've fixed that, and refactored a bit
further. Before, the lookup in the optimized code map was done in two different
ways (as HLoadNamedFields in the first case, and HLoadKeyed in the loop case).
Now, they are done in the same way, and code can be shared.
Thx,
--Michael

[Yang, 2014-03-10 14:00:23]

LGTM, except for the thing we talked about offline.

https://codereview.chromium.org/184103004/diff/80001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/184103004/diff/80001/src/code-stubs-hydrogen....
src/code-stubs-hydrogen.cc:103: // the function at map_index matches the given
native_context. builder is
Capitalize 'builder' :)

[mvstanton, 2014-03-12 13:28:18]

Thanks, Yang. Your idea of doing the check after the loop works nicely. Overall,
it's considerably more readable. Thanks again!
--Michael

https://codereview.chromium.org/184103004/diff/80001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/184103004/diff/80001/src/code-stubs-hydrogen....
src/code-stubs-hydrogen.cc:103: // the function at map_index matches the given
native_context. builder is
On 2014/03/10 14:00:23, Yang wrote:
> Capitalize 'builder' :)

Done.

[mvstanton, 2014-03-12 13:29:48]

Committed patchset #6 manually as r19846 (presubmit successful).