[compiler] Extend the functionality of CodeStubAssembler

[compiler] Extend the functionality of CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.

Committed: https://crrev.com/1f28904b2487a52d75e4c4fcd4a5a1abaf3456f5
Cr-Commit-Position: refs/heads/master@{#33664}

[danno, 2016-01-28 10:39:59]

danno@chromium.org changed reviewers:
+ bmeurer@chromium.org, mstarzinger@chromium.org

[danno, 2016-01-28 10:39:59]

PTAL

[danno, 2016-01-28 10:40:10]

Description was changed from

==========
[compiler] Extend the functionality CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.
==========

to

==========
[compiler] Extend the functionality of CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.
==========

[Michael Starzinger, 2016-02-01 16:04:12]

LGTM. Nice stuff, just nits to address.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
File src/compiler/code-stub-assembler.cc (right):

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.cc:174: USE(roots_array_start);
nit: Looks unimplemented, can we leave a TODO here?

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.cc:319: DCHECK(node->id() >= 0);
nit: DCHECK_GE

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.cc:366: DCHECK(value != nullptr);
nit: DCHECK_NOT_NULL

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
File src/compiler/code-stub-assembler.h (right):

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:83: DCHECK(impl_->value_ != nullptr);
nit: DCHECK_NOT_NULL

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:95: class Impl : public ZoneObject {
nit: Does this need to be a nested class within this nested class? Would it be
possible to have CSA::Variable and CSA::VariableImpl on the same nesting level?
The former would sat public as it is now and the latter could be made private
(and it's definition could even be put into the cc-file).

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:104: //
nit: A loon separator line could have a higher visual impact than the all-caps
here.

// ===========================================================================
// Base Assembler
//

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:156: // MACROS
nit: Likewise.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:181: friend class Label;
nit: Are these friend declarations needed? I was assuming nested classes have
the same access rights as members.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:198: ZoneVector<Variable::Impl*> vars_;
nit: s/vars/variables/

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/raw-machin...
File src/compiler/raw-machine-assembler.cc (right):

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/raw-machin...
src/compiler/raw-machine-assembler.cc:414: const Operator* new_op =
common()->Phi(rep, phi->InputCount());
nit: Please use CommonOperatorBuilder::ResizeMergeOrPhi for this.

https://codereview.chromium.org/1649723002/diff/40001/src/ic/stub-cache.h
File src/ic/stub-cache.h (right):

https://codereview.chromium.org/1649723002/diff/40001/src/ic/stub-cache.h#new...
src/ic/stub-cache.h:14: class CodeStubAssembler;
nit: This looks like a left-over from another piece of a bigger change. Can we
remove this change from this CL?

https://codereview.chromium.org/1649723002/diff/40001/src/ic/stub-cache.h#new...
src/ic/stub-cache.h:166: friend class compiler::CodeStubAssembler;
nit: This looks like a left-over from another piece of a bigger change. Can we
remove this change from this CL?

[danno, 2016-02-02 07:52:55]

Feedback addressed.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
File src/compiler/code-stub-assembler.cc (right):

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.cc:174: USE(roots_array_start);
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: Looks unimplemented, can we leave a TODO here?

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.cc:319: DCHECK(node->id() >= 0);
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: DCHECK_GE

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.cc:366: DCHECK(value != nullptr);
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: DCHECK_NOT_NULL

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
File src/compiler/code-stub-assembler.h (right):

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:83: DCHECK(impl_->value_ != nullptr);
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: DCHECK_NOT_NULL

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:95: class Impl : public ZoneObject {
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: Does this need to be a nested class within this nested class? Would it be
> possible to have CSA::Variable and CSA::VariableImpl on the same nesting
level?
> The former would sat public as it is now and the latter could be made private
> (and it's definition could even be put into the cc-file).

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:104: //
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: A loon separator line could have a higher visual impact than the all-caps
> here.
> 
> // ===========================================================================
> // Base Assembler
> //

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:156: // MACROS
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: Likewise.

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:181: friend class Label;
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: Are these friend declarations needed? I was assuming nested classes have
> the same access rights as members.

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:198: ZoneVector<Variable::Impl*> vars_;
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: s/vars/variables/

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/raw-machin...
File src/compiler/raw-machine-assembler.cc (right):

https://codereview.chromium.org/1649723002/diff/40001/src/compiler/raw-machin...
src/compiler/raw-machine-assembler.cc:414: const Operator* new_op =
common()->Phi(rep, phi->InputCount());
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: Please use CommonOperatorBuilder::ResizeMergeOrPhi for this.

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/ic/stub-cache.h
File src/ic/stub-cache.h (right):

https://codereview.chromium.org/1649723002/diff/40001/src/ic/stub-cache.h#new...
src/ic/stub-cache.h:14: class CodeStubAssembler;
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: This looks like a left-over from another piece of a bigger change. Can we
> remove this change from this CL?

Done.

https://codereview.chromium.org/1649723002/diff/40001/src/ic/stub-cache.h#new...
src/ic/stub-cache.h:166: friend class compiler::CodeStubAssembler;
On 2016/02/01 16:04:12, Michael Starzinger wrote:
> nit: This looks like a left-over from another piece of a bigger change. Can we
> remove this change from this CL?

Done.

[danno, 2016-02-02 07:53:39]

The CQ bit was checked by danno@chromium.org

[danno, 2016-02-02 07:53:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1649723002/#ps100001
(title: "Review feedback, fix tests")

[commit-bot: I haz the power, 2016-02-02 07:53:58]

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

[commit-bot: I haz the power, 2016-02-02 08:00:20]

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

[commit-bot: I haz the power, 2016-02-02 08:00:22]

Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[danno, 2016-02-02 08:03:22]

The CQ bit was checked by danno@chromium.org

[danno, 2016-02-02 08:03:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1649723002/#ps120001
(title: "Fix windows")

[commit-bot: I haz the power, 2016-02-02 08:03:27]

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

[commit-bot: I haz the power, 2016-02-02 08:09:33]

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

[commit-bot: I haz the power, 2016-02-02 08:09:33]

Try jobs failed on following builders:
  v8_win64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/2329)

[danno, 2016-02-02 08:42:37]

The CQ bit was checked by danno@chromium.org

[danno, 2016-02-02 08:42:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1649723002/#ps140001
(title: "Fix build")

[commit-bot: I haz the power, 2016-02-02 08:42:47]

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

[danno, 2016-02-02 08:56:52]

The CQ bit was checked by danno@chromium.org

[danno, 2016-02-02 08:56:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1649723002/#ps160001
(title: "Fix release build")

[commit-bot: I haz the power, 2016-02-02 08:57:03]

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

[commit-bot: I haz the power, 2016-02-02 09:42:21]

Description was changed from

==========
[compiler] Extend the functionality of CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.
==========

to

==========
[compiler] Extend the functionality of CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.
==========

[commit-bot: I haz the power, 2016-02-02 09:42:25]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-02-02 09:43:04]

Description was changed from

==========
[compiler] Extend the functionality of CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.
==========

to

==========
[compiler] Extend the functionality of CodeStubAssembler

After this change, the functionality of the CodeStubAssembler should be
sufficient to generate non-trivial stubs (e.g. the KeyedLoadIC) with control
flow, variables and probing of internal meta data structures.

Specifically this patch:

* introduces a Label class, which allows stubs to construct graphs that don't
  have linear control graphs.
* introduces a Variable class. Variables can be bound to Node* values at
  different points in a non-linear control flow graph. In conjunction with the
  Label machinery, the CodeStubAssembler ensures that Phi nodes are inserted at
  the "minimal" set of merge points.
* adds Tail calling support to other Stubs and to any arbitrary code whose
  interface can be described by a CallInterfaceDescriptor.
* provides new macros for accessing FixedArray elements that are optimized for
  use with Smi values.

Committed: https://crrev.com/1f28904b2487a52d75e4c4fcd4a5a1abaf3456f5
Cr-Commit-Position: refs/heads/master@{#33664}
==========

[commit-bot: I haz the power, 2016-02-02 09:43:05]

Patchset 9 (id:??) landed as
https://crrev.com/1f28904b2487a52d75e4c4fcd4a5a1abaf3456f5
Cr-Commit-Position: refs/heads/master@{#33664}