Issue 10832157: Unify handling of bailout IDs for property loads.

Issue 10832157: Unify handling of bailout IDs for property loads. (Closed)

Created:
8 years, 4 months ago by Sven Panne

Modified:
8 years, 4 months ago

Reviewers:
Michael Starzinger

CC:
v8-dev

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

Visibility:
Public.

More Reviews

Description

Unify handling of bailout IDs for property loads. Renamed Property::ReturnId to Property::LoadId, which describes its use more accurately. Compound assignments and CountOperations now consistently use Property::LoadId() instead of their own CompoundLoadId/CountId. Committed: https://code.google.com/p/v8/source/detail?r=12268

Patch Set 1 #

Patch Set 2 : Rebased. #

Total comments: 4

Patch Set 3 : Incorporated review comments. #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+28 lines, -33 lines)			Patch
M	src/arm/full-codegen-arm.cc	View	1	3 chunks	+4 lines, -4 lines	0 comments	Download
M	src/ast.h	View	1 2	7 chunks	+5 lines, -9 lines	0 comments	Download
M	src/ast.cc	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	src/hydrogen.cc	View	1 2	5 chunks	+7 lines, -7 lines	0 comments	Download
M	src/ia32/full-codegen-ia32.cc	View	1	3 chunks	+4 lines, -4 lines	0 comments	Download
M	src/mips/full-codegen-mips.cc	View	1	3 chunks	+4 lines, -4 lines	0 comments	Download
M	src/x64/full-codegen-x64.cc	View	1	3 chunks	+4 lines, -4 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Michael Starzinger

LGTM (with one comment). https://chromiumcodereview.appspot.com/10832157/diff/3001/src/ast.h File src/ast.h (right): https://chromiumcodereview.appspot.com/10832157/diff/3001/src/ast.h#newcode1813 src/ast.h:1813: BailoutId CountId() const { return ...

8 years, 4 months ago (2012-08-07 13:11:39 UTC) #2

Sven Panne

8 years, 4 months ago (2012-08-07 14:02:00 UTC) #3

Landing...

https://chromiumcodereview.appspot.com/10832157/diff/3001/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/10832157/diff/3001/src/ast.h#newcode1813
src/ast.h:1813: BailoutId CountId() const { return count_id_; }
On 2012/08/07 13:11:39, Michael Starzinger wrote:
> I think we can get rid of this BailoutId accessor as well ...

Done.

https://chromiumcodereview.appspot.com/10832157/diff/3001/src/ast.h#newcode1815
src/ast.h:1815: TypeFeedbackId CountBinOpFeedbackId() const { return
reuse(CountId()); }
On 2012/08/07 13:11:39, Michael Starzinger wrote:
> ... and just use the underlying AST id directly here.

Done.

Expand Messages | Collapse Messages