Split HPhase for Lithium and Hydrogen using common CompilationPhase base.

Split HPhase for Lithium and Hydrogen using common CompilationPhase base.

Add new base class CompilationPhase, which is the base for both HPhase, LPhase and LAllocatorPhase. HPhase is now for Hydrogen passes only, LPhase is for Lithium passes and LAllocatorPhase is for LAllocator phases.

R=svenpanne@chromium.org
BUG=

Committed: 15321

[danno, 2013-06-24 12:21:22]

https://codereview.chromium.org/17572011/diff/1/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/17572011/diff/1/src/compiler.cc#newcode370
src/compiler.cc:370: int64_t start_ticks = 0;
Seems like you have more than one patch in this CL. Isn't this another issue?

https://codereview.chromium.org/17572011/diff/1/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/17572011/diff/1/src/compiler.h#newcode634
src/compiler.h:634: Zone* zone_;
Why is there no ZoneScope in here? I don't see the advantage to moving the zone
into the phase unless it adds value. Until the phase manages the Zone's scope
(rather than the ad-hoc scopes in the individual phases, I would just leave it
in the info.

I also think you should either land this first and base your further phase
refactorings on this (so that they are subclasses), or, if you land this second,
include the "patch up" work in this CL to rebase the updated pases to subclass
from this class.

[Benedikt Meurer, 2013-06-24 13:04:08]

https://codereview.chromium.org/17572011/diff/1/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/17572011/diff/1/src/compiler.cc#newcode370
src/compiler.cc:370: int64_t start_ticks = 0;
On 2013/06/24 12:21:22, danno wrote:
> Seems like you have more than one patch in this CL. Isn't this another issue?

Done.

https://codereview.chromium.org/17572011/diff/1/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/17572011/diff/1/src/compiler.h#newcode634
src/compiler.h:634: Zone* zone_;
On 2013/06/24 12:21:22, danno wrote:
> Why is there no ZoneScope in here? I don't see the advantage to moving the
zone
> into the phase unless it adds value. Until the phase manages the Zone's scope
> (rather than the ad-hoc scopes in the individual phases, I would just leave it
> in the info.
> 
> I also think you should either land this first and base your further phase
> refactorings on this (so that they are subclasses), or, if you land this
second,
> include the "patch up" work in this CL to rebase the updated pases to subclass
> from this class.

Done.

[Sven Panne, 2013-06-25 11:48:20]

As discussed offline, we should split LPhase into a "real" Lithium phase and an
allocator phase, both inheriting from CompilationPhase.

[Benedikt Meurer, 2013-06-25 12:05:01]

On 2013/06/25 11:48:20, Sven Panne wrote:
> As discussed offline, we should split LPhase into a "real" Lithium phase and
an
> allocator phase, both inheriting from CompilationPhase.

Done.

[Sven Panne, 2013-06-25 12:18:38]

lgtm

[Benedikt Meurer, 2013-06-25 12:22:43]

Committed patchset #4 manually as r15321 (presubmit successful).