Issue 10918097: Allow smi comparisons to have constant operands.

Issue 10918097: Allow smi comparisons to have constant operands. (Closed)

Created:
8 years, 3 months ago by Vyacheslav Egorov (Google)

Modified:
8 years, 3 months ago

Reviewers:
Florian Schneider, srdjan

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Allow smi comparisons to have constant operands. Remove unnecessary temps reserved for them. Load constants directly in phi moves. Generate xor(reg, reg) for loading of 0 instead of mov. R=fschneider@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=11947

Patch Set 1 #

Total comments: 4

Created: 8 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+177 lines, -23 lines)			Patch
M	runtime/vm/flow_graph_allocator.cc	View	1 chunk	+8 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_compiler.cc	View	1 chunk	+42 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_compiler_ia32.h	View	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_compiler_ia32.cc	View	1 chunk	+6 lines, -1 line	0 comments	Download
M	runtime/vm/flow_graph_compiler_x64.h	View	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_compiler_x64.cc	View	1 chunk	+6 lines, -1 line	0 comments	Download
M	runtime/vm/intermediate_language.h	View	1 chunk	+2 lines, -0 lines	1 comment	Download
M	runtime/vm/intermediate_language.cc	View	1 chunk	+10 lines, -0 lines	0 comments	Download
M	runtime/vm/intermediate_language_ia32.cc	View	4 chunks	+47 lines, -10 lines	3 comments	Download
M	runtime/vm/intermediate_language_x64.cc	View	4 chunks	+48 lines, -11 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Florian Schneider

lgtm https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediate_language_ia32.cc File runtime/vm/intermediate_language_ia32.cc (right): https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediate_language_ia32.cc#newcode592 runtime/vm/intermediate_language_ia32.cc:592: Remove new line here?

8 years, 3 months ago (2012-09-06 13:17:30 UTC) #2

Vyacheslav Egorov (Google)

thanks https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediate_language_ia32.cc File runtime/vm/intermediate_language_ia32.cc (right): https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediate_language_ia32.cc#newcode592 runtime/vm/intermediate_language_ia32.cc:592: On 2012/09/06 13:17:30, Florian Schneider wrote: > Remove ...

8 years, 3 months ago (2012-09-06 13:24:44 UTC) #3

srdjan

8 years, 3 months ago (2012-09-06 13:32:55 UTC) #4

DBC

https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1341: void
EmitBranchOnValue(FlowGraphCompiler* compiler, bool result);
Please add comment explaining what flag 'result' means.

https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10918097/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:597: __ cmpl(right.reg(),
SmiConstantToImmediate(left.constant()));
I wonder if we should add assembly instruction that accept Smi instead of
Immediate.

Expand Messages | Collapse Messages