Issue 14159028: Fixed an issue with HConstant::InNewSpace() for parallel compilation.

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 14159028: Fixed an issue with HConstant::InNewSpace() for parallel compilation. (Closed)

Created:
7 years, 7 months ago by mvstanton

Modified:
7 years, 7 months ago

Reviewers:
Michael Starzinger

CC:
v8-dev

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

Visibility:
Public.

More Reviews

Description

Fixed an issue with HConstant::InNewSpace() for parallel compilation. Better to save the information about a constant handle being in new space at hydrogen time. BUG=

Patch Set 1 : Updates #

Patch Set 2 : Be able to recognize if running on the optimizer thread #

Created: 7 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+48 lines, -39 lines)			Patch
M	src/hydrogen-instructions.h	View		4 chunks	+9 lines, -13 lines	0 comments	Download
M	src/hydrogen-instructions.cc	View		2 chunks	+37 lines, -14 lines	0 comments	Download
M	src/ia32/lithium-ia32.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/optimizing-compiler-thread.h	View	1	3 chunks	+0 lines, -6 lines	0 comments	Download
M	src/optimizing-compiler-thread.cc	View	1	2 chunks	+0 lines, -4 lines	0 comments	Download
M	src/x64/lithium-x64.cc	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

mvstanton

Here are the changes we discussed. Care is taken to avoid dereferencing the handle if ...

7 years, 7 months ago (2013-04-29 12:52:47 UTC) #1

Michael Starzinger

7 years, 7 months ago (2013-04-29 17:28:21 UTC) #2

There are two reasons I am not particular fond of this change. The first is that
it tightly couples HConstant with the parallel compiler thread and whether we
are on of off that thread, as the changes in optimizing-compiler-thread.h show.
The second is that HConstant::Initialize sometimes uses an estimate for the
predicate and sometimes it exactly queries the current object location, it is
not obvious which is used when.

I have an alternative counter-proposal that always uses the exact object
location when an HConstant for a handle is constructed. When an existing
HConstant is copied, the original value is threaded through. And when an heap
number does not yet exists, we can guarantee is to not be in new-space. See
https://codereview.chromium.org/13977019/ for my proposal.

Expand Messages | Collapse Messages