[Ignition] Use binary operation feedback from Ignition to Crankshaft.

[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG.
This cl merges the feedback collected by ignition and FCG when passing to
crankshaft.

BUG=v8:4280
Committed: https://crrev.com/245e5b323c5f388187785004776dae0eae3c3f89
Cr-Commit-Position: refs/heads/master@{#39753}

[mythria, 2016-09-22 13:16:40]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 13:16:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 13:45:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 13:45:12]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-09-22 14:09:41]

mythria@chromium.org changed reviewers:
+ bmeurer@chromium.org, rmcilroy@chromium.org

[mythria, 2016-09-22 14:09:42]

Benedikt and Ross,

This cl is to pass feedback from ignition to turbofan for inlined functions when
feedback from full-codegen is not available. I also tried passing it for
non-inlined functions but it is not good for Kraken (regresses
audio-oscillator-orig by 80% and imaging-darkroom-orig by 30%). I will
investigate it too once I finish analyzing box2D. For now, we only pass ignition
feedback for inlined functions. This improves box2D by 40-50% and ray-trace by
10-20%. It does not regress anything else significantly. 

PTAL.

Thanks,
Mythri

https://codereview.chromium.org/2361043002/diff/1/src/type-feedback-vector-inl.h
File src/type-feedback-vector-inl.h (right):

https://codereview.chromium.org/2361043002/diff/1/src/type-feedback-vector-in...
src/type-feedback-vector-inl.h:127: return BinaryOperationHint::kNone;
Benedikt, I updated this function to pass None feedback when the slot is
uninitialized instead of Any. I thought the earlier implementation of passing
BinaryOperationHint::kAny was not intentional.If so, let me know I will add a
comment explaining why.

https://codereview.chromium.org/2361043002/diff/1/src/type-info.cc
File src/type-info.cc (right):

https://codereview.chromium.org/2361043002/diff/1/src/type-info.cc#newcode196
src/type-info.cc:196: AstType* CompareOpHintToType(CompareOperationHint hint) {
I actually wanted to move it to the CompareICNexus implementation, but I wasn't
sure, because type-feedback-vector does not know AstType. If that is not a
problem, I will move it to CompareICNexus.

[mythria, 2016-09-22 15:41:22]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 15:41:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 16:10:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 16:10:50]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2016-09-23 03:34:38]

LGTM. I'd really like to understand tho, what's causing the tankage with also
taking this feedback in general.

https://codereview.chromium.org/2361043002/diff/1/src/type-feedback-vector-inl.h
File src/type-feedback-vector-inl.h (right):

https://codereview.chromium.org/2361043002/diff/1/src/type-feedback-vector-in...
src/type-feedback-vector-inl.h:127: return BinaryOperationHint::kNone;
Yeah, this seems reasonable, defaulting to any is not a good idea.

[rmcilroy, 2016-09-23 07:56:58]

LGTM, but agree with Benedikt that we should figure out why this tanks
NavierStokes and Mandreel before landing.

nit - please capitalize Ignition and Crankshaft in title and description, and
also this should probably start with [Crankshaft] since it effects CS.

[mythria, 2016-09-26 10:27:25]

Description was changed from

==========
[Ignition] Use binary operation feedback from ignition when inlining functions
in crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used when optimizing using crankshaft.
This hits the performance, when trying to inline functions that are only
run using ignition. This cl passes the feedback collected by ignition to
crankshaft when feedback from full-codegen is not available. We only pass this
feedback for inlined functions. Using ignition feedback more generally
regresses a couple of benchmarks in Kraken. We need to investigate further
to be able to merge ignition and full-codegen feedback always.

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used when optimizing using crankshaft.
This hits the performance, when trying to optimize functions that are only
run using ignition. This cl merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 10:29:01]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used when optimizing using crankshaft.
This hits the performance, when trying to optimize functions that are only
run using ignition. This cl merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used when optimizing using crankshaft.
This hits the performance, when trying to optimize functions that did not
tier upto FCG. This cl merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 10:30:12]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used when optimizing using crankshaft.
This hits the performance, when trying to optimize functions that did not
tier upto FCG. This cl merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft.
This hits the performance, when trying to optimize functions that did not
tier upto FCG. This cl merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 10:30:43]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft.
This hits the performance, when trying to optimize functions that did not
tier upto FCG. This cl merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not
tier upto FCG. This cl
merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 10:31:16]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not
tier upto FCG. This cl
merges the feedback collected by ignition and
FCG when passing to crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not
tier upto FCG. This cl
merges the feedback collected by ignition and FCG when passing to crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 10:31:44]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not
tier upto FCG. This cl
merges the feedback collected by ignition and FCG when passing to crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG. This
cl
merges the feedback collected by ignition and FCG when passing to crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 10:32:15]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG. This
cl
merges the feedback collected by ignition and FCG when passing to crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG.
This cl merges the feedback collected by ignition and FCG when passing to
crankshaft. 

BUG=v8:4280
==========

[mythria, 2016-09-26 15:37:29]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 15:37:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 15:43:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 15:43:30]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[mythria, 2016-09-26 17:08:03]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 17:08:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 17:57:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 17:57:25]

Dry run: Try jobs failed on following builders:
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/13475)
  v8_linux64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng_triggered...)

[mythria, 2016-09-26 18:34:11]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 18:34:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 19:06:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 19:06:46]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-09-26 20:37:35]

I added a couple of fixes, one is a bug in my implementation which caused
regressions in Mandreel and Navier stokes and the other is a fix for the new
type feedback gathered by instance of.  Both these changes are in type-info.cc.
PTAL. Now there are no significant regressions in Kraken and Octane. PdfJS in
octane regresses about 5-10%, but that is a bit flaky. 

Details about performance regressions:
 The regressions in Mandreel and Navier stokes was because of a bug in
translating CompareOperationHint to AstType. I did not add a case for kNumber
and hence it was going to default and getting converted to kAny. I fixed it in
the new patch. 

The regressions in audio-oscillator in Kraken would be fixed by another cl
(https://codereview.chromium.org/2369043002/).  These regressions was a kind of
side effect from this cl. There is one function in Kraken that gets optimized
using Turbofan OSR and when we deopt from turbo fan it gets compiled using
crankshaft. When we don't pass feedback from ignition (ie. without this patch)
the crankshaft deopts immediately because it does not have binary op feedback.
Then the code gets compiled to full-codegen and then tiers up to crankshaft. In
this process, enough type feedback is gathered about LoadICs in that function
and hence generates better code. 

When we pass feedback from ignition the crankshaft code does not deopt and hence
executes less efficient optimized code.  The LoadICs are in pre-monomorphic
state in this case. I am not quite sure, but I  think pre-monomorphic should be
treated as uninitialized, and that would cause the function to deopt and
generate better code during the next optimization.

https://codereview.chromium.org/2361043002/diff/80001/src/type-info.cc
File src/type-info.cc (right):

https://codereview.chromium.org/2361043002/diff/80001/src/type-info.cc#newcod...
src/type-info.cc:221: case BinaryOperationHint::kNumberOrOddball:
We only collect feedback about Number. We change it to NumberOrOddball when
translating BinaryOpFeedback to BinaryOperaionHint. Here, if I convert it to
AstType::NumberOrOddball, it will cause deopts in crankshaft in some cases. I am
not sure, but I think we can add kNumber and kNumberOrOddball to
BinaryOperationHint too, just like CompareOperation.

https://codereview.chromium.org/2361043002/diff/80001/src/type-info.cc#newcod...
src/type-info.cc:240: if (!info->IsCode()) {
I had to change this check because now instance of collects type feedback in
general slot. And that feedback is not collected here.  It is used when
optimizing and not when analyzing type info.

[Benedikt Meurer, 2016-09-27 03:44:30]

> Details about performance regressions:
>  The regressions in Mandreel and Navier stokes was because of a bug in
> translating CompareOperationHint to AstType. I did not add a case for kNumber
> and hence it was going to default and getting converted to kAny. I fixed it in
> the new patch. 

That's one of the reason why we try to avoid default at any cost.

> The regressions in audio-oscillator in Kraken would be fixed by another cl
> (https://codereview.chromium.org/2369043002/).  These regressions was a kind
of
> side effect from this cl. There is one function in Kraken that gets optimized
> using Turbofan OSR and when we deopt from turbo fan it gets compiled using
> crankshaft. When we don't pass feedback from ignition (ie. without this patch)
> the crankshaft deopts immediately because it does not have binary op feedback.
> Then the code gets compiled to full-codegen and then tiers up to crankshaft.
In
> this process, enough type feedback is gathered about LoadICs in that function
> and hence generates better code. 
> 
> When we pass feedback from ignition the crankshaft code does not deopt and
hence
> executes less efficient optimized code.  The LoadICs are in pre-monomorphic
> state in this case. I am not quite sure, but I  think pre-monomorphic should
be
> treated as uninitialized, and that would cause the function to deopt and
> generate better code during the next optimization.

Yeah this sounds a lot like https://codereview.chromium.org/2309943003/, I'm
still not sure if we should land it or not (my CL is about TF only, but we can
do the same for CS).

> https://codereview.chromium.org/2361043002/diff/80001/src/type-info.cc
> File src/type-info.cc (right):
> 
>
https://codereview.chromium.org/2361043002/diff/80001/src/type-info.cc#newcod...
> src/type-info.cc:221: case BinaryOperationHint::kNumberOrOddball:
> We only collect feedback about Number. We change it to NumberOrOddball when
> translating BinaryOpFeedback to BinaryOperaionHint. Here, if I convert it to
> AstType::NumberOrOddball, it will cause deopts in crankshaft in some cases. I
am
> not sure, but I think we can add kNumber and kNumberOrOddball to
> BinaryOperationHint too, just like CompareOperation.

This might be related to ToBinaryOperationHint in type-hint-analyzer.cc, where
we have to interpret INT32 feedback from the BinaryOpIC differently depending on
the actual binary operation. But yeah, I guess having separate kNumber case in
BinaryOperationHint makes sense; it'll help TurboFan to generate less code for
the checks when coming from Ignition. The only reason we didn't have that there
so far was that the BinaryOpIC would never produce this feedback, the NUMBER
state always allows some oddballs.

[mythria, 2016-09-27 07:56:05]

The CQ bit was checked by mythria@chromium.org

[mythria, 2016-09-27 07:56:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2361043002/#ps80001
(title: "Removed iostream from type-info.cc")

[commit-bot: I haz the power, 2016-09-27 07:56:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 08:03:25]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG.
This cl merges the feedback collected by ignition and FCG when passing to
crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG.
This cl merges the feedback collected by ignition and FCG when passing to
crankshaft. 

BUG=v8:4280
==========

[commit-bot: I haz the power, 2016-09-27 08:03:26]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-27 08:03:49]

Description was changed from

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG.
This cl merges the feedback collected by ignition and FCG when passing to
crankshaft. 

BUG=v8:4280
==========

to

==========
[Ignition] Use binary operation feedback from Ignition to Crankshaft.

Ignition collects type feedback for binary and compare operations in type
feedback vector and FCG uses Binary/CompareOpICs to collect type feedback.
The feedback collected by ignition is not used by crankshaft. This hits the
performance, when trying to optimize functions that did not tier upto FCG.
This cl merges the feedback collected by ignition and FCG when passing to
crankshaft.

BUG=v8:4280

Committed: https://crrev.com/245e5b323c5f388187785004776dae0eae3c3f89
Cr-Commit-Position: refs/heads/master@{#39753}
==========

[commit-bot: I haz the power, 2016-09-27 08:03:50]

Patchset 5 (id:??) landed as
https://crrev.com/245e5b323c5f388187785004776dae0eae3c3f89
Cr-Commit-Position: refs/heads/master@{#39753}