Add graph intrinsics for many math library functions

Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.

This is my original CL including the CheckClass instruction.

R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/f5bc8fa353772bb66537cc2862f7f4b0a3f6cfbb

[Cutch, 2016-03-21 19:46:48]

Description was changed from

==========
Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.
==========

to

==========
Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.
==========

[Cutch, 2016-03-21 19:46:49]

johnmccutchan@google.com changed reviewers:
+ fschneider@google.com

[Cutch, 2016-03-21 19:47:44]

Description was changed from

==========
Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.
==========

to

==========
Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.

This is my original CL including the CheckClass instruction.
==========

[Florian Schneider, 2016-03-22 09:21:03]

I did not realize that we assumed the inputs of AddUnboxInstr to be checked.
This can be solved by adding a parameter is_checked.

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:323: Definition* AddUnboxInstr(Representation rep,
Value* value) {
Definition* AddUnboxInstr(Representation rep, Value* value, bool is_checked) {

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:327: // when building intrinsics.
if (is_checked) {
  ...

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:329:
CompileType::FromCid(CidForRepresentation(rep))));
}

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:333: Definition* AddUnboxInstr(Representation rep,
Definition* boxed) {
Definition* AddUnboxInstr(Representation rep, Definition* boxed, bool
is_checked) {

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:965: Definition* unboxed_value =
builder->AddCheckedUnbox(kUnboxedDouble, value);
builder->AddCheckedUnbox(kUnboxedDouble, value, /*is_checked =*/ false);

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:1059: Definition* value_a = builder.AddParameter(1);
s/1/2

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:1060: Definition* value_b = builder.AddParameter(2);
s/2/1

The index is relative to SP, so the last argument (value_b) gets index 1.

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:1062: Definition* unboxed_a =
builder.AddCheckedUnbox(kUnboxedDouble, value_a);
Definition* unboxed_a = builder.AddCheckedUnbox(kUnboxedDouble, value_a, false);

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:1063: Definition* unboxed_b =
builder.AddCheckedUnbox(kUnboxedDouble, value_b);
Definition* unboxed_b = builder.AddCheckedUnbox(kUnboxedDouble, value_b, false);

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:1065: // C's atan2 expects the arguments in the
opposite order.
Comment not needed (see comment above)

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:1068: args->Add(new Value(unboxed_b));
Swap this line with the one below.

[Cutch, 2016-03-22 16:09:55]

PTAL

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/1821783002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:323: Definition* AddUnboxInstr(Representation rep,
Value* value) {
On 2016/03/22 09:21:02, Florian Schneider wrote:
> Definition* AddUnboxInstr(Representation rep, Value* value, bool is_checked) {

Acknowledged.

[Florian Schneider, 2016-03-22 16:26:21]

LGTM!

https://codereview.chromium.org/1821783002/diff/40001/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/1821783002/diff/40001/runtime/vm/intrinsifier...
runtime/vm/intrinsifier.cc:298: Definition* AddCheckedUnbox(Representation rep,
Definition* boxed) {
Remove if not needed anymore.

[Cutch, 2016-03-22 16:28:53]

Description was changed from

==========
Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.

This is my original CL including the CheckClass instruction.
==========

to

==========
Add graph intrinsics for many math library functions

- sin, cos, tan, asin, acos, atan, and atan2.

This is my original CL including the CheckClass instruction.

R=fschneider@google.com

Committed:
https://github.com/dart-lang/sdk/commit/f5bc8fa353772bb66537cc2862f7f4b0a3f6cfbb
==========

[Cutch, 2016-03-22 16:28:57]

Committed patchset #4 (id:60001) manually as
f5bc8fa353772bb66537cc2862f7f4b0a3f6cfbb (presubmit successful).