Issue 8960004: Avoid embedding new space objects into code objects in the lithium gap resolver.

Issue 8960004: Avoid embedding new space objects into code objects in the lithium gap resolver. (Closed)

Created:
9 years ago by Vyacheslav Egorov (Chromium)

Modified:
9 years ago

Reviewers:
danno

CC:
v8-dev

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

Visibility:
Public.

More Reviews

Description

Avoid embedding new space objects into code objects in the lithium gap resolver. R=danno@chromium.org BUG=http://crbug.com/108296 TEST=test/mjsunit/regress/regress-108296.js Committed: http://code.google.com/p/v8/source/detail?r=10301

Patch Set 1 #

Total comments: 2

Created: 9 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+137 lines, -28 lines)			Patch
M	src/arm/lithium-codegen-arm.h	View	1 chunk	+3 lines, -0 lines	0 comments	Download
M	src/arm/lithium-codegen-arm.cc	View	1 chunk	+12 lines, -0 lines	0 comments	Download
M	src/arm/lithium-gap-resolver-arm.cc	View	1 chunk	+14 lines, -3 lines	0 comments	Download
M	src/arm/macro-assembler-arm.h	View	1 chunk	+8 lines, -0 lines	0 comments	Download
M	src/ia32/lithium-codegen-ia32.h	View	2 chunks	+8 lines, -2 lines	2 comments	Download
M	src/ia32/lithium-codegen-ia32.cc	View	5 chunks	+7 lines, -17 lines	0 comments	Download
M	src/ia32/lithium-gap-resolver-ia32.cc	View	1 chunk	+14 lines, -4 lines	0 comments	Download
M	src/ia32/macro-assembler-ia32.h	View	1 chunk	+8 lines, -0 lines	0 comments	Download
M	src/x64/lithium-gap-resolver-x64.cc	View	2 chunks	+3 lines, -2 lines	0 comments	Download
M	src/x64/macro-assembler-x64.h	View	1 chunk	+8 lines, -0 lines	0 comments	Download
A	test/mjsunit/regress/regress-108296.js	View	1 chunk	+52 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

danno

Great catch. LGTM with nit. http://codereview.chromium.org/8960004/diff/1/src/ia32/lithium-codegen-ia32.h File src/ia32/lithium-codegen-ia32.h (right): http://codereview.chromium.org/8960004/diff/1/src/ia32/lithium-codegen-ia32.h#newcode84 src/ia32/lithium-codegen-ia32.h:84: return Immediate(ToInteger32(LConstantOperand::cast(op))); ::cast is ...

9 years ago (2011-12-23 09:09:22 UTC) #2

Vyacheslav Egorov (Chromium)

9 years ago (2011-12-23 10:36:36 UTC) #3

Thanks!

I will land and backport.

http://codereview.chromium.org/8960004/diff/1/src/ia32/lithium-codegen-ia32.h
File src/ia32/lithium-codegen-ia32.h (right):

http://codereview.chromium.org/8960004/diff/1/src/ia32/lithium-codegen-ia32.h...
src/ia32/lithium-codegen-ia32.h:84: return
Immediate(ToInteger32(LConstantOperand::cast(op)));
On 2011/12/23 09:09:23, danno wrote:
> ::cast is redundant?

I don't think so, 

ToInteger takes LConstantOperand*, 
ToInteger32Immediate LOperand*

(I introduced this helper only to avoid writing this cast everywhere)

Expand Messages | Collapse Messages