[compiler] Put shared function marking behind a flag

[compiler] Put shared function marking behind a flag

This lets us investigate regressions caused by this marking while
letting others continue their work without being impacted.

BUG=v8:5512
Committed: https://crrev.com/f97b28f8b9678f9529acb21551a10f466a0b722e
Cr-Commit-Position: refs/heads/master@{#40563}

[Leszek Swirski, 2016-10-24 16:58:30]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 16:58:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-24 17:01:13]

leszeks@chromium.org changed reviewers:
+ mstarzinger@chromium.org, mvstanton@chromium.org

[Leszek Swirski, 2016-10-24 17:01:14]

Putting this behind a flag while we analyze the performance regressions.

[commit-bot: I haz the power, 2016-10-24 17:29:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-24 17:29:43]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-10-25 10:38:24]

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

https://codereview.chromium.org/2446673002/diff/1/src/compiler.cc#newcode977
src/compiler.cc:977: function->shared()->was_marked_for_optimization()) {
nit: This should actually never happen when the flag is not set. Can we instead
just put a DCHECK(FLAG_mark_shared_functions) inside the block?

https://codereview.chromium.org/2446673002/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2446673002/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:278: DEFINE_BOOL(mark_shared_functions, false,
nit: The word "mark" is very much used for the GC marking, it makes it sound
like this flag is GC related. How would you feel about renaming this flag to
"FLAG_optimize_shared_functions" instead?

[Leszek Swirski, 2016-10-25 10:54:41]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 10:54:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-25 10:56:10]

Thanks Michi, all done.

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

https://codereview.chromium.org/2446673002/diff/1/src/compiler.cc#newcode977
src/compiler.cc:977: function->shared()->was_marked_for_optimization()) {
On 2016/10/25 10:38:24, Michael Starzinger wrote:
> nit: This should actually never happen when the flag is not set. Can we
instead
> just put a DCHECK(FLAG_mark_shared_functions) inside the block?

Good point, done.

https://codereview.chromium.org/2446673002/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2446673002/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:278: DEFINE_BOOL(mark_shared_functions, false,
On 2016/10/25 10:38:24, Michael Starzinger wrote:
> nit: The word "mark" is very much used for the GC marking, it makes it sound
> like this flag is GC related. How would you feel about renaming this flag to
> "FLAG_optimize_shared_functions" instead?

Sure, done, though I may rename it again in the followup "tier up" CL  (to, say,
tier_up_shared_functions).

[commit-bot: I haz the power, 2016-10-25 11:39:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-25 11:39:48]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-10-25 12:05:13]

LGTM from my end.

https://codereview.chromium.org/2446673002/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2446673002/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:278: DEFINE_BOOL(mark_shared_functions, false,
On 2016/10/25 10:56:10, Leszek Swirski wrote:
> On 2016/10/25 10:38:24, Michael Starzinger wrote:
> > nit: The word "mark" is very much used for the GC marking, it makes it sound
> > like this flag is GC related. How would you feel about renaming this flag to
> > "FLAG_optimize_shared_functions" instead?
> 
> Sure, done, though I may rename it again in the followup "tier up" CL  (to,
say,
> tier_up_shared_functions).

Acknowledged. Yes, "tier up" also works for me. Totally up to you.

[Leszek Swirski, 2016-10-25 12:25:00]

The CQ bit was checked by leszeks@chromium.org

[commit-bot: I haz the power, 2016-10-25 12:25:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 12:34:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-17 22:12:36]

Description was changed from

==========
[compiler] Put shared function marking behind a flag

This lets us investigate regressions caused by this marking while
letting others continue their work without being impacted.

BUG=v8:5512
==========

to

==========
[compiler] Put shared function marking behind a flag

This lets us investigate regressions caused by this marking while
letting others continue their work without being impacted.

BUG=v8:5512

Committed: https://crrev.com/f97b28f8b9678f9529acb21551a10f466a0b722e
Cr-Commit-Position: refs/heads/master@{#40563}
==========

[commit-bot: I haz the power, 2016-11-17 22:12:38]

Patchset 2 (id:??) landed as
https://crrev.com/f97b28f8b9678f9529acb21551a10f466a0b722e
Cr-Commit-Position: refs/heads/master@{#40563}