Merge sin(a), cos(a) into one instruction. TODO: Implement for ARM and MIPS.

Merge sin(a), cos(a) into one instruction. On IA32 use x87 fsincos operation, on x64 call runtime routine whihc merges sin/cos intos sincos.
TODO: Implement for ARM and MIPS.

R=fschneider@google.com

Committed: https://code.google.com/p/dart/source/detail?r=30827

[Florian Schneider, 2013-12-03 14:12:04]

LGTM.

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:361: optimizer.TryOptimizePatterns();
Is it necessary to run this twice, here and below? Also, I'm not sure that the
TODO above is still applicable: We do track environment uses properly now,
right?

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:322: const intptr_t other_result_ix =
Equivalent, but shorter:

const intptr_t other_result_ix = (curr_result_ix == 0) ? 1 : 0;

or even just flip the LSB:

const intptr_t other_result_ix = curr_result_ix ^ 1;

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:378: (other_op->kind() ==
MethodRecognizer::kMathSin) ? 0 : 1;
Equivalent, but shorter:

const intptr_t other_result_ix = (curr_result_ix == 0) ? 1 : 0;

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:381: ZoneGrowableArray<Value*>* args = new
ZoneGrowableArray<Value*>(2);
s/2/1/

because only one argument is added below.

https://codereview.chromium.org/92433002/diff/90001/tests/language/arithmetic...
File tests/language/arithmetic_test.dart (right):

https://codereview.chromium.org/92433002/diff/90001/tests/language/arithmetic...
tests/language/arithmetic_test.dart:438: static void testSinCos() {
Maybe test a few more values like PI, PI/2 as done for the other tests for sin
and cos

[srdjan, 2013-12-03 18:10:01]

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:361: optimizer.TryOptimizePatterns();
On 2013/12/03 14:12:04, Florian Schneider wrote:
> Is it necessary to run this twice, here and below? Also, I'm not sure that the
> TODO above is still applicable: We do track environment uses properly now,
> right?

The one below, after CSE is important. I will eliminate the one here. Adding
comment:
Run after CSE in order to have more opportunity to merge instructions that have
same inputs.

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:378: (other_op->kind() ==
MethodRecognizer::kMathSin) ? 0 : 1;
On 2013/12/03 14:12:04, Florian Schneider wrote:
> Equivalent, but shorter:
> 
> const intptr_t other_result_ix = (curr_result_ix == 0) ? 1 : 0;

I used to have that, but found it harder to read.  Factored the code out so that
it is linked with the code generation.

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:381: ZoneGrowableArray<Value*>* args = new
ZoneGrowableArray<Value*>(2);
On 2013/12/03 14:12:04, Florian Schneider wrote:
> s/2/1/
> 
> because only one argument is added below.

Done.

https://codereview.chromium.org/92433002/diff/90001/tests/language/arithmetic...
File tests/language/arithmetic_test.dart (right):

https://codereview.chromium.org/92433002/diff/90001/tests/language/arithmetic...
tests/language/arithmetic_test.dart:438: static void testSinCos() {
On 2013/12/03 14:12:04, Florian Schneider wrote:
> Maybe test a few more values like PI, PI/2 as done for the other tests for sin
> and cos

Done.

[srdjan, 2013-12-03 21:01:38]

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/92433002/diff/90001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:361: optimizer.TryOptimizePatterns();
Correction: the early one is needed to mark left shifts as truncating. Left it
here,

On 2013/12/03 18:10:01, srdjan wrote:
> On 2013/12/03 14:12:04, Florian Schneider wrote:
> > Is it necessary to run this twice, here and below? Also, I'm not sure that
the
> > TODO above is still applicable: We do track environment uses properly now,
> > right?
> 
> The one below, after CSE is important. I will eliminate the one here. Adding
> comment:
> Run after CSE in order to have more opportunity to merge instructions that
have
> same inputs.

[srdjan, 2013-12-03 22:00:45]

Committed patchset #9 manually as r30827 (presubmit successful).