Reorder KeyedStoreIC MISS code to avoid unnecessary compilation.

Reorder KeyedStoreIC MISS code to avoid unnecessary compilation.

We can set the property in the MISS handler before organizing our handlers
for element-based keyed stores. Since the property set may fail with an
exception, this saves work.

BUG=

Committed: https://crrev.com/dd0cde0e4872655a376f7aba7d0ba6251a343a88
Cr-Commit-Position: refs/heads/master@{#30444}

[mvstanton, 2015-08-28 12:13:17]

mvstanton@chromium.org changed reviewers:
+ ishell@chromium.org

[mvstanton, 2015-08-28 12:13:17]

Hi Igor,
Here is the change we were discussing. I front-load the work that might change
after the property set.

This will make vector-based stores way easier.
Thanks,
--Michael

[Igor Sheludko, 2015-08-28 12:25:18]

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2166
src/ic/ic.cc:2166: if (object->IsJSObject()) {
You need all these only if use_ic is true.

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2171
src/ic/ic.cc:2171: isolate()->heap()->sloppy_arguments_elements_map();
I think "&& !is_sloppy(language_mode()" should also be here.

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2181
src/ic/ic.cc:2181: ASSIGN_RETURN_ON_EXCEPTION(isolate(), store_handle,
Why did you drop "if (store_handle.is_null())" ? What about adding DCHECK
instead?

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2188
src/ic/ic.cc:2188: if (sloppy_arguments_elements && !is_sloppy(language_mode()))
{
"&& !is_sloppy(language_mode()" should be included into
sloppy_arguments_elements definition.

[mvstanton, 2015-08-28 12:31:17]

Thanks for the comments Igor, addressed 'em...
Michael

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2166
src/ic/ic.cc:2166: if (object->IsJSObject()) {
On 2015/08/28 12:25:18, Igor Sheludko wrote:
> You need all these only if use_ic is true.

Indeed, adding this check.

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2171
src/ic/ic.cc:2171: isolate()->heap()->sloppy_arguments_elements_map();
On 2015/08/28 12:25:18, Igor Sheludko wrote:
> I think "&& !is_sloppy(language_mode()" should also be here.

No problemo.

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2181
src/ic/ic.cc:2181: ASSIGN_RETURN_ON_EXCEPTION(isolate(), store_handle,
On 2015/08/28 12:25:18, Igor Sheludko wrote:
> Why did you drop "if (store_handle.is_null())" ? What about adding DCHECK
> instead?

I saw that we can't get here with it non_null.
I'll add the DCHECK as you suggest.

https://codereview.chromium.org/1308073010/diff/20001/src/ic/ic.cc#newcode2188
src/ic/ic.cc:2188: if (sloppy_arguments_elements && !is_sloppy(language_mode()))
{
On 2015/08/28 12:25:18, Igor Sheludko wrote:
> "&& !is_sloppy(language_mode()" should be included into
> sloppy_arguments_elements definition.

Okay, will do.

[Igor Sheludko, 2015-08-28 12:37:26]

lgtm

[mvstanton, 2015-08-28 12:42:02]

The CQ bit was checked by mvstanton@chromium.org

[commit-bot: I haz the power, 2015-08-28 12:42:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1308073010/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1308073010/40001

[commit-bot: I haz the power, 2015-08-28 13:13:15]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-28 13:13:24]

Patchset 3 (id:??) landed as
https://crrev.com/dd0cde0e4872655a376f7aba7d0ba6251a343a88
Cr-Commit-Position: refs/heads/master@{#30444}

[Igor Sheludko, 2015-08-28 14:32:52]

On 2015/08/28 13:13:24, commit-bot: I haz the power wrote:
> Patchset 3 (id:??) landed as
> https://crrev.com/dd0cde0e4872655a376f7aba7d0ba6251a343a88
> Cr-Commit-Position: refs/heads/master@{#30444}

It looks like you committed something completely different than I lgtmed :) I
mean the REBASE thing.

[mvstanton, 2015-08-28 15:22:36]

Ah, no, don't worry that was a mistake after patching this CL into another
issue. That crazy REBASE didn't get checked in. I'll delete the patchset, sorry
for the confusion!

[mvstanton, 2015-08-28 15:22:43]

Patchset #4 (id:60001) has been deleted