[turbofan] Add DebugBreak machine operator and support

[turbofan] Add DebugBreak machine operator and support

Committed: https://crrev.com/78b1585f1d148b2bbbc27f870efbe4e2787bbffb
Cr-Commit-Position: refs/heads/master@{#36355}

[danno, 2016-05-19 08:34:58]

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

[danno, 2016-05-19 08:34:59]

PTAL

[Michael Starzinger, 2016-05-19 08:42:11]

https://codereview.chromium.org/1995543003/diff/40001/src/compiler/opcodes.h
File src/compiler/opcodes.h (right):

https://codereview.chromium.org/1995543003/diff/40001/src/compiler/opcodes.h#...
src/compiler/opcodes.h:23: V(DebugBreak)            \
As discussed offline: Could this be turned into a machine operator instead of a
control operator? Otherwise we need to handle it in all control flow analysis
phases (e.g. scheduler and friends).

[danno, 2016-05-19 08:46:48]

Feedback addressed, please take another look.

https://codereview.chromium.org/1995543003/diff/40001/src/compiler/opcodes.h
File src/compiler/opcodes.h (right):

https://codereview.chromium.org/1995543003/diff/40001/src/compiler/opcodes.h#...
src/compiler/opcodes.h:23: V(DebugBreak)            \
On 2016/05/19 08:42:11, Michael Starzinger wrote:
> As discussed offline: Could this be turned into a machine operator instead of
a
> control operator? Otherwise we need to handle it in all control flow analysis
> phases (e.g. scheduler and friends).

Done.

[Michael Starzinger, 2016-05-19 08:53:24]

LGTM with comments.

https://codereview.chromium.org/1995543003/diff/100001/src/code-stub-assemble...
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/1995543003/diff/100001/src/code-stub-assemble...
src/code-stub-assembler.cc:26: #if defined(DEBUG)
nit: Should we guard this by FLAG_debug_code instead if the DEBUG define,
similar to the macro-assembler? Either is fine with me, just wanted to mention
it.

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
File src/compiler/code-assembler.cc (right):

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
src/compiler/code-assembler.cc:535: void CodeAssembler::DebugBreak() {
raw_assembler_->DebugBreak(); }
nit: Likewise.

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
File src/compiler/code-assembler.h (right):

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
src/compiler/code-assembler.h:212: void DebugBreak();
nit: Can we move this up or down a few lines? Right now it sits right in between
the control flow operators (i.e. between Branch and Switch). How about line 205
instead?

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/machine-o...
File src/compiler/machine-operator.cc (right):

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/machine-o...
src/compiler/machine-operator.cc:533: 1, 1, 0, 1, 0) {}
I assume the effect inputs are there in the hope of constraining it in the
effect chain. For now this operator is only used in scheduled graph. Would you
be fine with making this a "pure" operator for now instead (i.e. just adding it
to PURE_OP_LIST). Constraining it in the effect chain would require further
testing.

[danno, 2016-05-19 09:05:04]

Feedback addressed, landing

https://codereview.chromium.org/1995543003/diff/100001/src/code-stub-assemble...
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/1995543003/diff/100001/src/code-stub-assemble...
src/code-stub-assembler.cc:26: #if defined(DEBUG)
On 2016/05/19 08:53:23, Michael Starzinger wrote:
> nit: Should we guard this by FLAG_debug_code instead if the DEBUG define,
> similar to the macro-assembler? Either is fine with me, just wanted to mention
> it.

As discussed, let's leave it as-is.

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
File src/compiler/code-assembler.cc (right):

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
src/compiler/code-assembler.cc:535: void CodeAssembler::DebugBreak() {
raw_assembler_->DebugBreak(); }
On 2016/05/19 08:53:23, Michael Starzinger wrote:
> nit: Likewise.

Done.

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
File src/compiler/code-assembler.h (right):

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/code-asse...
src/compiler/code-assembler.h:212: void DebugBreak();
On 2016/05/19 08:53:23, Michael Starzinger wrote:
> nit: Can we move this up or down a few lines? Right now it sits right in
between
> the control flow operators (i.e. between Branch and Switch). How about line
205
> instead?

Done.

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/machine-o...
File src/compiler/machine-operator.cc (right):

https://codereview.chromium.org/1995543003/diff/100001/src/compiler/machine-o...
src/compiler/machine-operator.cc:533: 1, 1, 0, 1, 0) {}
On 2016/05/19 08:53:23, Michael Starzinger wrote:
> I assume the effect inputs are there in the hope of constraining it in the
> effect chain. For now this operator is only used in scheduled graph. Would you
> be fine with making this a "pure" operator for now instead (i.e. just adding
it
> to PURE_OP_LIST). Constraining it in the effect chain would require further
> testing.

Done.

[danno, 2016-05-19 09:05:11]

The CQ bit was checked by danno@chromium.org

[danno, 2016-05-19 09:05:11]

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/1995543003/#ps120001
(title: "Review feedback")

[commit-bot: I haz the power, 2016-05-19 09:05:14]

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

[commit-bot: I haz the power, 2016-05-19 09:46:41]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-05-19 09:48:04]

Description was changed from

==========
[turbofan] Add DebugBreak machine operator and support
==========

to

==========
[turbofan] Add DebugBreak machine operator and support

Committed: https://crrev.com/78b1585f1d148b2bbbc27f870efbe4e2787bbffb
Cr-Commit-Position: refs/heads/master@{#36355}
==========

[commit-bot: I haz the power, 2016-05-19 09:48:05]

Patchset 7 (id:??) landed as
https://crrev.com/78b1585f1d148b2bbbc27f870efbe4e2787bbffb
Cr-Commit-Position: refs/heads/master@{#36355}