Issue 7273066: Simplify EmitCallIC.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 7273066: Simplify EmitCallIC. (Closed)

Created:
9 years, 5 months ago by Sven Panne

Modified:
9 years, 5 months ago

Reviewers:
fschneider, Mads Ager (chromium)

CC:
v8-dev

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

Visibility:
Public.

Description

Simplify EmitCallIC. Move the identification of the corresponding StatsCounter where it belongs, namely into the Code class. Use the null pattern to signal "no inlined Smi code". Removed a few unused counters. Committed: http://code.google.com/p/v8/source/detail?r=8470

Patch Set 1 #

Total comments: 6

Patch Set 2 : '' #

Created: 9 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+77 lines, -245 lines)			Patch
M	src/arm/full-codegen-arm.cc	View	1	7 chunks	+20 lines, -62 lines	0 comments	Download
M	src/full-codegen.h	View		1 chunk	+0 lines, -7 lines	0 comments	Download
M	src/ia32/full-codegen-ia32.cc	View	1	7 chunks	+19 lines, -55 lines	0 comments	Download
M	src/mips/full-codegen-mips.cc	View	1	7 chunks	+19 lines, -61 lines	0 comments	Download
M	src/v8-counters.h	View	1	1 chunk	+0 lines, -4 lines	0 comments	Download
M	src/x64/full-codegen-x64.cc	View	1	7 chunks	+19 lines, -56 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Mads Ager (chromium)

LGTM http://codereview.chromium.org/7273066/diff/1/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/7273066/diff/1/src/objects-inl.h#newcode928 src/objects-inl.h:928: (*reinterpret_cast<const int*>(FIELD_ADDR(p, offset))) Unrelated change?

9 years, 5 months ago (2011-06-29 12:58:42 UTC) #2

fschneider

9 years, 5 months ago (2011-06-29 13:00:39 UTC) #3

http://codereview.chromium.org/7273066/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/7273066/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:865: EmitCallIC(ic, RelocInfo::CODE_TARGET,
clause->CompareId());
I'd still prefer statically helping to never forget emitting the patch info.
Dynamically checking with ASSERT only works for debug mode, and only if our
tests hit all the places. That's why the parameter was introduced.

Plus there is an extra line at each call site.

http://codereview.chromium.org/7273066/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:4203: if (StatsCounter* counter =
ic->Counter(isolate())) {
Do we really need those counters at all? We don't count loads/stores in
Crankshaft generated code, so I think they aren't very useful anymore. Maybe we
should just get rid of them.

Expand Messages | Collapse Messages