Issue 12090021: Foundation for the use of informative definitions in Crankshaft.

Issue 12090021: Foundation for the use of informative definitions in Crankshaft. (Closed)

Created:
7 years, 10 months ago by Massi

Modified:
7 years, 10 months ago

Reviewers:
Jakob Kummerow

CC:
v8-dev

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

Visibility:
Public.

More Reviews

Description

Foundation for the use of informative definitions in Crankshaft. Committed: https://code.google.com/p/v8/source/detail?r=13543

Patch Set 2 : Rebased on bleeding edge. #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+121 lines, -57 lines)			Patch
M	src/arm/lithium-codegen-arm.cc	View	1	1 chunk	+0 lines, -2 lines	0 comments	Download
M	src/code-stubs-hydrogen.cc	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	src/hydrogen.h	View		2 chunks	+3 lines, -1 line	0 comments	Download
M	src/hydrogen.cc	View	1	12 chunks	+39 lines, -16 lines	0 comments	Download
M	src/hydrogen-instructions.h	View	1	5 chunks	+51 lines, -23 lines	0 comments	Download
M	src/hydrogen-instructions.cc	View		1 chunk	+22 lines, -0 lines	0 comments	Download
M	src/ia32/lithium-codegen-ia32.cc	View	1	3 chunks	+4 lines, -9 lines	0 comments	Download
M	src/mips/lithium-codegen-mips.cc	View	1	1 chunk	+0 lines, -2 lines	0 comments	Download
M	src/x64/lithium-codegen-x64.cc	View	1	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Jakob Kummerow

LGTM with nits. https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h#newcode686 src/hydrogen-instructions.h:686: bool IsInformativelDefinition() { nit: s/IsInformativelDefinition/IsInformativeDefinition/ https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h#newcode698 ...

7 years, 10 months ago (2013-01-29 10:33:01 UTC) #2

Massi

7 years, 10 months ago (2013-01-29 15:39:00 UTC) #3

Addressed comments and rebased...

https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:686: bool IsInformativelDefinition() {
On 2013/01/29 10:33:01, Jakob wrote:
> nit: s/IsInformativelDefinition/IsInformativeDefinition/

Done.

https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:698: ? RedefinedOperand()->ActualValue() : this;
On 2013/01/29 10:33:01, Jakob wrote:
> nit: preferred formatting: 
>     return IsInformativeDefinition() ? RedefinedOperand()->ActualValue()
>                                      : this;

Done.

https://codereview.chromium.org/12090021/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:4458: ? Representation::Integer32() :
Representation::Tagged();
On 2013/01/29 10:33:01, Jakob wrote:
> nit: preferred formatting:
>     return r.IsInteger32() ? Representation::Integer32()
>                            : Representation::Tagged();

Done.

https://codereview.chromium.org/12090021/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/12090021/diff/1/src/hydrogen.cc#newcode4059
src/hydrogen.cc:4059: HPhase phase("H_Apply actual values", this);
On 2013/01/29 10:33:01, Jakob wrote:
> nit: "H_Restore actual values" (matching the method's name)

Done.

Expand Messages | Collapse Messages