[interpreter] VisitForTest for bytecode generator

[interpreter] VisitForTest for bytecode generator

Adds TestResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n

Committed: https://crrev.com/935340a4c51bc35f472cef4a1d978882e3040c98
Cr-Commit-Position: refs/heads/master@{#38634}

[klaasb, 2016-08-11 16:27:16]

klaasb@google.com changed reviewers:
+ mstarzinger@chromium.org, rmcilroy@chromium.org

[klaasb, 2016-08-11 16:27:16]

Ross, Michi, please take a look.

The names VisitFor"Control" and the related "Control"things I don't particularly
care about, but i couldn't decide between that and
VisitFor{Test,Conditional,Branch,Jump,..}. This is what FCG uses, ASTGB uses
VisitForTest. Any strong opinions?

[rmcilroy, 2016-08-12 11:15:27]

Looks great overall. Bunch of comments but mostly just nits.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:539: enum class
BytecodeGenerator::ControlFallthrough { THEN, ELSE, NONE };
Just define this in the header. Also, enum class uses "kConstant" notation
(i.e., kThen, kElse, kNone)

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:542: class
BytecodeGenerator::BytecodeLabels {
How about moving this to bytecode-label.h/cc?

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:558: std::vector<BytecodeLabel> labels_;
use ZoneVector (which means you will need to pass the zone to the constructor).

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:578: DCHECK(has_result_);
No need for these DCHECKS - the set_result_identified will DCHECK if it is
called twice.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:590: void SetNoResult() {
How about SetResultConsumedByTest

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:596: bool HasResultValue() { return
has_result_; }
ResultConsumedByTest() (and invert the boolean)

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:605: ControlFallthrough
InvertedFallthrough() {
These are all getters, so lowercase - then_labels, else_labels, fallthrough and
inverted_fallthrough. Also add "const" to all of them.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1069: BytecodeLabel end_label;
nit - move end_label into inner else branch too.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1082: BytecodeLabels then_labels;
nit - BytecodeLabels then_labels, else_labels;

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1094: if (stmt->HasElseStatement()) {
could you keep the if/else form:

if (stmt->HasElseStatement()) {
  builder()->Jump(&end_label);
  else_labels.Bind(builder());
  Visit(stmt->else_statement());
} else {
  else_labels.Bind(builder());
}

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1672: BytecodeLabels else_labels;
nit - BytecodeLabels then_labels, else_labels;

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1973: // this will generate a new register
when within a RegisterResultScope
Thanks for updating this. How about:

We need to load the variable into the accumulator, even when in a
VisitForRegisterScope, in order to avoid register aliasing if subsequent 
expressions assign to the same variable.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1984: // this will generate a new register
when within a RegisterResultScope
ditto

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2845: if (execution_result()->IsTest()) {
Could you also add the IsEffect shortcut here (just VisitForEffect the value).

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2847:
static_cast<ControlResultScope*>(execution_result());
Could you add an AsTest() function with DCHECKS to ExecutionResult. Also, this
should be reinterpreter_cast<...>

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2848: VisitForControl(expr->expression(),
control_result->ElseLabels(),
nit - comment that you are swapping the control flow.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3139: if (left->ToBooleanIsTrue()) {
I think you could do:
 if (left->ToBooleanIsTrue() || right->ToBooleanIsTrue) {
   builder()->Jump(control_result->NewThenLabel());
 } else if (left->ToBooleanIsFalse() && right->ToBooleanIsFalse()) {
   builder()->Jump(control_result->NewElseLabel());
 ...

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3153: // no need to visit right
Just remove this comment (ditto below)

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3156: } else {
I know this isn't your code, but we should also be able to do:
else if (left->ToBooleanIsFalse()) {
  VisitForAccumulatorValue(right);
}

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3175: if (left->ToBooleanIsFalse()) {
Same suggestion (for AND) as above for OR.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3189: // no need to visit right
ditto

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3190: if (left->ToBooleanIsFalse()) {
Same suggestion as above.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3450: // to make sure that all temporary
registers are returned before generating
/s/to/To

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3452: // otherwise dead registers might be
materialized
fullstop

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3466: default:
Make this explicitly case ControlFallthrough::kNone

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.h:40: class ControlResultScope;
Let's go with TestControlScope and VisitForTest. That way we won't get confused
between ControlResultScope and ControlScope, and also being in sync with
ast-graph-builder is the way to go since TF is the future ;).

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
File test/cctest/test-func-name-inference.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
test/cctest/test-func-name-inference.cc:305: "var x = 0;\n"
Were these changes intended?

[klaasb, 2016-08-12 16:00:13]

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:539: enum class
BytecodeGenerator::ControlFallthrough { THEN, ELSE, NONE };
On 2016/08/12 11:15:25, rmcilroy wrote:
> Just define this in the header. Also, enum class uses "kConstant" notation
> (i.e., kThen, kElse, kNone)

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:542: class
BytecodeGenerator::BytecodeLabels {
On 2016/08/12 11:15:26, rmcilroy wrote:
> How about moving this to bytecode-label.h/cc?

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:558: std::vector<BytecodeLabel> labels_;
On 2016/08/12 11:15:25, rmcilroy wrote:
> use ZoneVector (which means you will need to pass the zone to the
constructor).

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:578: DCHECK(has_result_);
On 2016/08/12 11:15:26, rmcilroy wrote:
> No need for these DCHECKS - the set_result_identified will DCHECK if it is
> called twice.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:590: void SetNoResult() {
On 2016/08/12 11:15:25, rmcilroy wrote:
> How about SetResultConsumedByTest

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:596: bool HasResultValue() { return
has_result_; }
On 2016/08/12 11:15:26, rmcilroy wrote:
> ResultConsumedByTest() (and invert the boolean)

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:605: ControlFallthrough
InvertedFallthrough() {
On 2016/08/12 11:15:26, rmcilroy wrote:
> These are all getters, so lowercase - then_labels, else_labels, fallthrough
and
> inverted_fallthrough. Also add "const" to all of them.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1069: BytecodeLabel end_label;
On 2016/08/12 11:15:26, rmcilroy wrote:
> nit - move end_label into inner else branch too.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1082: BytecodeLabels then_labels;
On 2016/08/12 11:15:26, rmcilroy wrote:
> nit - BytecodeLabels then_labels, else_labels;

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1094: if (stmt->HasElseStatement()) {
On 2016/08/12 11:15:26, rmcilroy wrote:
> could you keep the if/else form:
> 
> if (stmt->HasElseStatement()) {
>   builder()->Jump(&end_label);
>   else_labels.Bind(builder());
>   Visit(stmt->else_statement());
> } else {
>   else_labels.Bind(builder());
> }

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1672: BytecodeLabels else_labels;
On 2016/08/12 11:15:26, rmcilroy wrote:
> nit - BytecodeLabels then_labels, else_labels;

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1973: // this will generate a new register
when within a RegisterResultScope
On 2016/08/12 11:15:26, rmcilroy wrote:
> Thanks for updating this. How about:
> 
> We need to load the variable into the accumulator, even when in a
> VisitForRegisterScope, in order to avoid register aliasing if subsequent 
> expressions assign to the same variable.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1984: // this will generate a new register
when within a RegisterResultScope
On 2016/08/12 11:15:25, rmcilroy wrote:
> ditto

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2845: if (execution_result()->IsTest()) {
On 2016/08/12 11:15:26, rmcilroy wrote:
> Could you also add the IsEffect shortcut here (just VisitForEffect the value).

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2847:
static_cast<ControlResultScope*>(execution_result());
On 2016/08/12 11:15:26, rmcilroy wrote:
> Could you add an AsTest() function with DCHECKS to ExecutionResult. Also, this
> should be reinterpreter_cast<...>

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2848: VisitForControl(expr->expression(),
control_result->ElseLabels(),
On 2016/08/12 11:15:26, rmcilroy wrote:
> nit - comment that you are swapping the control flow.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3139: if (left->ToBooleanIsTrue()) {
On 2016/08/12 11:15:26, rmcilroy wrote:
> I think you could do:
>  if (left->ToBooleanIsTrue() || right->ToBooleanIsTrue) {
>    builder()->Jump(control_result->NewThenLabel());
>  } else if (left->ToBooleanIsFalse() && right->ToBooleanIsFalse()) {
>    builder()->Jump(control_result->NewElseLabel());
>  ...

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3153: // no need to visit right
On 2016/08/12 11:15:26, rmcilroy wrote:
> Just remove this comment (ditto below)

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3156: } else {
On 2016/08/12 11:15:25, rmcilroy wrote:
> I know this isn't your code, but we should also be able to do:
> else if (left->ToBooleanIsFalse()) {
>   VisitForAccumulatorValue(right);
> }

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3175: if (left->ToBooleanIsFalse()) {
On 2016/08/12 11:15:26, rmcilroy wrote:
> Same suggestion (for AND) as above for OR.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3189: // no need to visit right
On 2016/08/12 11:15:26, rmcilroy wrote:
> ditto

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3190: if (left->ToBooleanIsFalse()) {
On 2016/08/12 11:15:25, rmcilroy wrote:
> Same suggestion as above.

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3450: // to make sure that all temporary
registers are returned before generating
On 2016/08/12 11:15:26, rmcilroy wrote:
> /s/to/To

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3452: // otherwise dead registers might be
materialized
On 2016/08/12 11:15:26, rmcilroy wrote:
> fullstop
> 

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:3466: default:
On 2016/08/12 11:15:26, rmcilroy wrote:
> Make this explicitly case ControlFallthrough::kNone

Done.

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/2242463002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.h:40: class ControlResultScope;
On 2016/08/12 11:15:26, rmcilroy wrote:
> Let's go with TestControlScope and VisitForTest. That way we won't get
confused
> between ControlResultScope and ControlScope, and also being in sync with
> ast-graph-builder is the way to go since TF is the future ;).

Done.

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
File test/cctest/test-func-name-inference.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
test/cctest/test-func-name-inference.cc:305: "var x = 0;\n"
On 2016/08/12 11:15:26, rmcilroy wrote:
> Were these changes intended?

I introduced these to keep the intention of these tests the same. With the
previous version with the new VisitForTest changes, one branch would get
optimized away and the test would fail.
This would need updating again, if we ever propagate constants via variables
while generating code.

[klaasb, 2016-08-12 16:00:39]

Description was changed from

==========
[interpreter] VisitForControl for bytecode generator

Adds ControlResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n
==========

to

==========
[interpreter] VisitForTest for bytecode generator

Adds TestResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n
==========

[klaasb, 2016-08-12 16:13:10]

The CQ bit was checked by klaasb@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 16:13:16]

Dry run: 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-08-12 16:15:27]

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

[commit-bot: I haz the power, 2016-08-12 16:15:28]

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

[klaasb, 2016-08-12 16:24:11]

The CQ bit was checked by klaasb@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 16:24:16]

Dry run: 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-08-12 16:44:59]

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

[commit-bot: I haz the power, 2016-08-12 16:45:00]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-08-15 09:37:11]

LGTM, but please comment on the test-function-name-inference test failure
without the change.

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
File test/cctest/test-func-name-inference.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
test/cctest/test-func-name-inference.cc:305: "var x = 0;\n"
On 2016/08/12 16:00:13, klaasb wrote:
> On 2016/08/12 11:15:26, rmcilroy wrote:
> > Were these changes intended?
> 
> I introduced these to keep the intention of these tests the same. With the
> previous version with the new VisitForTest changes, one branch would get
> optimized away and the test would fail.
> This would need updating again, if we ever propagate constants via variables
> while generating code.

How does the test fail? It seems like the function name should still be inferred
correctly even if one of the branches is optimized away.

https://codereview.chromium.org/2242463002/diff/60001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242463002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:595: bool result_consumed_;
nit - result_consumed_by_test_

[klaasb, 2016-08-15 11:53:04]

See rationale on test-func-name-inference.cc changes.

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
File test/cctest/test-func-name-inference.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
test/cctest/test-func-name-inference.cc:305: "var x = 0;\n"
On 2016/08/15 09:37:11, rmcilroy wrote:
> How does the test fail? It seems like the function name should still be
inferred
> correctly even if one of the branches is optimized away.

If we skip generating bytecode for the branch, we never see the function literal
and also don't generate a SharedFunctionInfo.
CheckFunctionName tries to find the SharedFunctionInfo closest to and containing
"return 1", but only find the containing anonymous function.
As far as I understand, we don't need the SharedFunctionInfo and we should be
fine optimizing this. Is that correct?

This is just a simple fix for the test for now.
Though in general, these two tests will always depend on the optimizations we
do, unless we insert something unoptimizable/non-constant on which to branch.
How about Math.random()? It won't make these tests non-deterministic, since
they're not actually executed.

https://codereview.chromium.org/2242463002/diff/60001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2242463002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:595: bool result_consumed_;
On 2016/08/15 09:37:11, rmcilroy wrote:
> nit - result_consumed_by_test_

Done.

[rmcilroy, 2016-08-15 12:37:19]

Still LGTM

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
File test/cctest/test-func-name-inference.cc (right):

https://codereview.chromium.org/2242463002/diff/20001/test/cctest/test-func-n...
test/cctest/test-func-name-inference.cc:305: "var x = 0;\n"
On 2016/08/15 11:53:04, klaasb wrote:
> On 2016/08/15 09:37:11, rmcilroy wrote:
> > How does the test fail? It seems like the function name should still be
> inferred
> > correctly even if one of the branches is optimized away.
> 
> If we skip generating bytecode for the branch, we never see the function
literal
> and also don't generate a SharedFunctionInfo.
> CheckFunctionName tries to find the SharedFunctionInfo closest to and
containing
> "return 1", but only find the containing anonymous function.
> As far as I understand, we don't need the SharedFunctionInfo and we should be
> fine optimizing this. Is that correct?
> 
> This is just a simple fix for the test for now.
> Though in general, these two tests will always depend on the optimizations we
> do, unless we insert something unoptimizable/non-constant on which to branch.
> How about Math.random()? It won't make these tests non-deterministic, since
> they're not actually executed.

Ahh I see, so it's not actually the function name inference which is incorrect,
just the test code that tries to extract the SFI at a particular source
position. It that case this looks fine, thanks for the explanation.

Re: future optimizations - we will never be able to do the optimizations you
suggest due to debugging (we need to have breakable source positions after the
local was updated) so that won't be an issue, this is fine as-is.

[klaasb, 2016-08-15 12:47:17]

The CQ bit was checked by klaasb@google.com

[commit-bot: I haz the power, 2016-08-15 12:47:21]

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-08-15 13:10:48]

Description was changed from

==========
[interpreter] VisitForTest for bytecode generator

Adds TestResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n
==========

to

==========
[interpreter] VisitForTest for bytecode generator

Adds TestResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n
==========

[commit-bot: I haz the power, 2016-08-15 13:10:49]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-15 13:11:10]

Description was changed from

==========
[interpreter] VisitForTest for bytecode generator

Adds TestResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n
==========

to

==========
[interpreter] VisitForTest for bytecode generator

Adds TestResultScope and uses it to directly jump/fall through to the
correct branch in expressions used as branch conditions.
Should enable nicer TurboFan-graphs for easier control-flow
transformations in the future.

BUG=v8:4280
LOG=n

Committed: https://crrev.com/935340a4c51bc35f472cef4a1d978882e3040c98
Cr-Commit-Position: refs/heads/master@{#38634}
==========

[commit-bot: I haz the power, 2016-08-15 13:11:11]

Patchset 5 (id:??) landed as
https://crrev.com/935340a4c51bc35f472cef4a1d978882e3040c98
Cr-Commit-Position: refs/heads/master@{#38634}