Issue 8827015: Implement two-argument check inline cache.

Issue 8827015: Implement two-argument check inline cache. (Closed)

Created:
9 years ago by srdjan

Modified:
9 years ago

Reviewers:
regis

CC:
reviews_dartlang.org, vm-dev_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Implement two-arguments-check inline cache. Use it first for Smi binary operations. Committed: https://code.google.com/p/dart/source/detail?r=2345

Patch Set 5 : '' #

Created: 9 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+338 lines, -115 lines)			Patch
M	vm/code_generator.h	View	1 2 3 4	1 chunk	+2 lines, -1 line	0 comments	Download
M	vm/code_generator.cc	View	1 2 3 4	3 chunks	+67 lines, -16 lines	0 comments	Download
M	vm/code_generator_ia32.h	View	1 2 3 4	1 chunk	+4 lines, -1 line	0 comments	Download
M	vm/code_generator_ia32.cc	View	1 2 3 4	9 chunks	+40 lines, -13 lines	0 comments	Download
M	vm/code_patcher_ia32_test.cc	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	vm/ic_data.h	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	vm/ic_data.cc	View	1 2 3 4	1 chunk	+0 lines, -2 lines	0 comments	Download
M	vm/opt_code_generator_ia32.cc	View	1 2 3 4	8 chunks	+89 lines, -33 lines	0 comments	Download
M	vm/stub_code.h	View	1 2 3 4	2 chunks	+4 lines, -1 line	0 comments	Download
M	vm/stub_code_arm.cc	View	1 2 3 4	1 chunk	+7 lines, -2 lines	0 comments	Download
M	vm/stub_code_ia32.cc	View	1 2 3 4	4 chunks	+116 lines, -42 lines	0 comments	Download
M	vm/stub_code_x64.cc	View	1 2 3 4	1 chunk	+7 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

regis

LGTM http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.cc File vm/code_generator_ia32.cc (right): http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.cc#newcode1070 vm/code_generator_ia32.cc:1070: 1); Declaring a constant as for kNumArguments and ...

9 years ago (2011-12-09 23:08:03 UTC) #3

srdjan

9 years ago (2011-12-12 17:19:33 UTC) #4

http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.cc
File vm/code_generator_ia32.cc (right):

http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.cc#ne...
vm/code_generator_ia32.cc:1070: 1);
On 2011/12/09 23:08:03, regis wrote:
> Declaring a constant as for kNumArguments and kNoArgumentNames would make the
> code easier to read.

Added const int kNumArgumentsChecked

http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.cc#ne...
vm/code_generator_ia32.cc:1116: 1);
On 2011/12/09 23:08:03, regis wrote:
> ditto and ditto for all other instances below.

Done.

http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.h
File vm/code_generator_ia32.h (right):

http://codereview.chromium.org/8827015/diff/9005/vm/code_generator_ia32.h#new...
vm/code_generator_ia32.h:174: intptr_t num_checks);
On 2011/12/09 23:08:03, regis wrote:
> Would it be possible to spare this additional argument and derive it from
> function_name and num_arguments?
> 
> The name "num_checks" does not say much. What kind of checks?

num_checks -> num_args_checked, added comment as well.

I do no think that is simple to derive num_args_checked from the function_name
and num_arguments. It seems more reliable if that knowledge is embedded in the
appropriate visitor method.

Expand Messages | Collapse Messages