Issue 9338002: Fixed a bug in isNegate intrinsified code, fixed negation for doubles (handling of negative zeros...

Issue 9338002: Fixed a bug in isNegate intrinsified code, fixed negation for doubles (handling of negative zeros... (Closed)

Created:
8 years, 10 months ago by srdjan

Modified:
8 years, 10 months ago

Reviewers:
regis

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Fixed a bug in isNegate intrinsified code, fixed negation for doubles (handling of negative zeros). Added tests. Fix issue 1554. Factored out parts of X86Decoder::InstructionDecode in order to satisfy lint (500 lines max per function). Committed: https://code.google.com/p/dart/source/detail?r=3971

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 4

Patch Set 4 : '' #

Created: 8 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+82 lines, -45 lines)			Patch
M	runtime/lib/double.dart	View	1 2 3	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/disassembler_ia32.cc	View	1 2 3	3 chunks	+61 lines, -40 lines	0 comments	Download
M	runtime/vm/intrinsifier_ia32.cc	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/vm/opt_code_generator_ia32.cc	View	1 2 3	1 chunk	+2 lines, -3 lines	0 comments	Download
M	tests/language/language.status	View	1 2 3	2 chunks	+4 lines, -1 line	0 comments	Download
M	tests/language/src/ArithmeticTest.dart	View	1 2 3	1 chunk	+10 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

regis

LGTM https://chromiumcodereview.appspot.com/9338002/diff/2003/runtime/lib/double.dart File runtime/lib/double.dart (right): https://chromiumcodereview.appspot.com/9338002/diff/2003/runtime/lib/double.dart#newcode52 runtime/lib/double.dart:52: // -0.0 is canonicalized by the VM's parser, ...

8 years, 10 months ago (2012-02-06 21:45:18 UTC) #2

srdjan

8 years, 10 months ago (2012-02-06 22:07:24 UTC) #3

https://chromiumcodereview.appspot.com/9338002/diff/2003/runtime/lib/double.dart
File runtime/lib/double.dart (right):

https://chromiumcodereview.appspot.com/9338002/diff/2003/runtime/lib/double.d...
runtime/lib/double.dart:52: // -0.0 is canonicalized by the VM's parser,
therefore no cycles.
On 2012/02/06 21:45:19, regis wrote:
> I do not understand this comment. Does it mean that 'this' cannot be -0.0? I
> guess it can, since you are calling isNegative() below.

Yes, it can, but it is important that -0.0 is not translated into literal 0.0
and negate operation, otherwise we would have a cycle where negate calls
negate..

https://chromiumcodereview.appspot.com/9338002/diff/2003/runtime/vm/disassemb...
File runtime/vm/disassembler_ia32.cc (right):

https://chromiumcodereview.appspot.com/9338002/diff/2003/runtime/vm/disassemb...
runtime/vm/disassembler_ia32.cc:1009: uword addr =
*reinterpret_cast<uword*>((*data)+1);
On 2012/02/06 21:45:19, regis wrote:
> Extra pair of parenthesis around *data is not consistent with other
occurrences
> below.

Done.

Expand Messages | Collapse Messages