[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in pipeline.cc

[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org

Committed: https://crrev.com/7498f49fcbf81c399bb4a27c9e2e391d39ff8705
Cr-Commit-Position: refs/heads/master@{#35787}

[titzer, 2016-04-20 16:29:18]

Description was changed from

==========
[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in
pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org
==========

to

==========
[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in
pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org
==========

[titzer, 2016-04-20 16:29:18]

titzer@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[titzer, 2016-04-20 16:29:27]

On 2016/04/20 15:53:21, ahaas wrote:

+mstarzinger

[Michael Starzinger, 2016-04-20 16:59:08]

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.c...
src/compiler/pipeline.cc:1418: bool
Pipeline::ExecuteWasmCompilation(CallDescriptor* descriptor) {
This duplicates quite a lot of complex logic. We could add a private
Pipeline::ScheduleGraph and a private Pipeline::GenerateCode method. These two
could then be called from the two new phases separately or from
Pipeline::ScheduleAndGenerateCode in sequence.

Or am I missing something and the two pipeline are intentionally separated and
we intend to make them diverge?

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h
File src/compiler/pipeline.h (right):

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h...
src/compiler/pipeline.h:66: void InitializeWasmCompilation(Zone* pipeline_zone,
ZonePool* zone_pool,
Not actionable for this CL, but I think we should really turn this into an
OptimizedCompileJob eventually. Maybe a TODO in that regard makes sense already.
WDYT?

[Michael Starzinger, 2016-04-20 20:46:32]

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h
File src/compiler/pipeline.h (right):

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h...
src/compiler/pipeline.h:71: Handle<Code> ScheduleAndGenerateCode(CallDescriptor*
call_descriptor);
Also, this seems to no longer be called from the outside, please make it private
again.

[ahaas, 2016-04-21 07:59:16]

On 2016/04/20 at 20:46:32, mstarzinger wrote:
> https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h
> File src/compiler/pipeline.h (right):
> 
>
https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h...
> src/compiler/pipeline.h:71: Handle<Code>
ScheduleAndGenerateCode(CallDescriptor* call_descriptor);
> Also, this seems to no longer be called from the outside, please make it
private again.

Done.

[ahaas, 2016-04-21 07:59:16]

On 2016/04/20 at 20:46:32, mstarzinger wrote:
> https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h
> File src/compiler/pipeline.h (right):
> 
>
https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h...
> src/compiler/pipeline.h:71: Handle<Code>
ScheduleAndGenerateCode(CallDescriptor* call_descriptor);
> Also, this seems to no longer be called from the outside, please make it
private again.

Done.

[ahaas, 2016-04-21 07:59:44]

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.c...
src/compiler/pipeline.cc:1418: bool
Pipeline::ExecuteWasmCompilation(CallDescriptor* descriptor) {
On 2016/04/20 at 16:59:08, Michael Starzinger wrote:
> This duplicates quite a lot of complex logic. We could add a private
Pipeline::ScheduleGraph and a private Pipeline::GenerateCode method. These two
could then be called from the two new phases separately or from
Pipeline::ScheduleAndGenerateCode in sequence.
> 
> Or am I missing something and the two pipeline are intentionally separated and
we intend to make them diverge?

I use the same code for the new functions and ScheduleAndGenerateCode now.

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h
File src/compiler/pipeline.h (right):

https://codereview.chromium.org/1904723002/diff/20001/src/compiler/pipeline.h...
src/compiler/pipeline.h:71: Handle<Code> ScheduleAndGenerateCode(CallDescriptor*
call_descriptor);
On 2016/04/20 at 20:46:32, Michael Starzinger wrote:
> Also, this seems to no longer be called from the outside, please make it
private again.

Done.

[Michael Starzinger, 2016-04-21 08:15:44]

LGTM.

[titzer, 2016-04-21 08:17:29]

On 2016/04/21 08:15:44, Michael Starzinger wrote:
> LGTM.

lgtm

[ahaas, 2016-04-26 09:23:02]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-04-26 09:23:02]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org,
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1904723002/#ps60001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-04-26 09:23:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1904723002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1904723002/60001

[commit-bot: I haz the power, 2016-04-26 09:44:44]

Description was changed from

==========
[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in
pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org
==========

to

==========
[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in
pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org
==========

[commit-bot: I haz the power, 2016-04-26 09:44:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-26 09:45:07]

Description was changed from

==========
[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in
pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org
==========

to

==========
[wasm] Introduce ExecuteWasmCompilation and FinalizeWasmCompilation in
pipeline.cc

With these two functions we can split the compilation pipeline for wasm
functions just before the code generation.

R=titzer@chromium.org, clemensh@chromium.org

Committed: https://crrev.com/7498f49fcbf81c399bb4a27c9e2e391d39ff8705
Cr-Commit-Position: refs/heads/master@{#35787}
==========

[commit-bot: I haz the power, 2016-04-26 09:45:09]

Patchset 4 (id:??) landed as
https://crrev.com/7498f49fcbf81c399bb4a27c9e2e391d39ff8705
Cr-Commit-Position: refs/heads/master@{#35787}

[Benedikt Meurer, 2016-04-26 11:47:44]

bmeurer@chromium.org changed reviewers:
+ bmeurer@chromium.org

[Benedikt Meurer, 2016-04-26 11:47:45]

https://codereview.chromium.org/1904723002/diff/60001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1904723002/diff/60001/src/compiler/pipeline.c...
src/compiler/pipeline.cc:1408: data_->Destroy();
This at least leaks the ostringstream that you added, maybe even more.