Issue 9320023: Intrinisifed more core library methods: isNaN, isNegative, cos, sin.

Issue 9320023: Intrinisifed more core library methods: isNaN, isNegative, cos, sin. (Closed)

Created:
8 years, 10 months ago by srdjan

Modified:
8 years, 10 months ago

Reviewers:
siva

CC:
reviews_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Intrinisifed more core library methods: isNaN, isNegative, cos, sin. TODO: inline those methods in otpimized code. Committed: https://code.google.com/p/dart/source/detail?r=3869

Patch Set 1 #

Total comments: 2

Patch Set 2 : '' #

Total comments: 5

Patch Set 3 : '' #

Created: 8 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+192 lines, -10 lines)			Patch
M	runtime/vm/assembler_ia32.h	View	1 2	2 chunks	+3 lines, -0 lines	0 comments	Download
M	runtime/vm/assembler_ia32.cc	View	1 2	2 chunks	+16 lines, -0 lines	0 comments	Download
M	runtime/vm/assembler_ia32_test.cc	View	1 2	2 chunks	+31 lines, -0 lines	0 comments	Download
M	runtime/vm/intrinsifier_ia32.cc	View	1 2	4 chunks	+101 lines, -0 lines	0 comments	Download
M	tests/language/language.status	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download
M	tests/language/src/IntrinsifiedMethodsTest.dart	View	1 2	1 chunk	+31 lines, -4 lines	0 comments	Download
M	tests/language/src/MathVMTest.dart	View	1 2	4 chunks	+7 lines, -5 lines	0 comments	Download
M	tests/language/src/Unary2Test.dart	View	1 2	2 chunks	+2 lines, -1 line	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

siva

LGTM (I am not very familiar with rules for a double being Nan or negative ...

8 years, 10 months ago (2012-02-02 00:04:26 UTC) #2

srdjan

8 years, 10 months ago (2012-02-02 19:07:53 UTC) #3

https://chromiumcodereview.appspot.com/9320023/diff/1/runtime/vm/intrinsifier...
File runtime/vm/intrinsifier_ia32.cc (right):

https://chromiumcodereview.appspot.com/9320023/diff/1/runtime/vm/intrinsifier...
runtime/vm/intrinsifier_ia32.cc:898: __ Bind(&is_nan);
On 2012/02/02 00:04:26, asiva wrote:
> Why do you want two labels to the same location? Why not just use is_nan for
> both cases.

Getting rid of is_nan label. I think it makes more sense than jumping to is_nan
if value is >=0

https://chromiumcodereview.appspot.com/9320023/diff/3002/runtime/vm/assembler...
File runtime/vm/assembler_ia32.cc (right):

https://chromiumcodereview.appspot.com/9320023/diff/3002/runtime/vm/assembler...
runtime/vm/assembler_ia32.cc:592: 
On 2012/02/02 00:04:26, asiva wrote:
> Do we need equivalent versions for x64 also> maybe Regis can do it in another
> CL.

x64 is not intrinsifying anything, and there is no use of that instructions. I
think we should move it once we need it.

https://chromiumcodereview.appspot.com/9320023/diff/3002/tests/language/src/I...
File tests/language/src/IntrinsifiedMethodsTest.dart (right):

https://chromiumcodereview.appspot.com/9320023/diff/3002/tests/language/src/I...
tests/language/src/IntrinsifiedMethodsTest.dart:1: // Copyright (c) 2011, the
Dart project authors.  Please see the AUTHORS file
On 2012/02/02 00:04:26, asiva wrote:
> 2012, some other files may also have to be updated.

Changed all modified test files 2011 -> 2012

Expand Messages | Collapse Messages