Fix bugs in IfBuilder and improve functionality

Fix bugs in IfBuilder and improve functionality

R=mstarzinger@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=14327

[Michael Starzinger, 2013-04-18 10:30:21]

LGTM (with a few asserts and nits).

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.cc#newcode661
src/hydrogen.cc:661: builder_->CreateBasicBlock(env->Copy());
nit: Should fit into one line now. Also three more occurrences below.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.cc#newcode1455
src/hydrogen.cc:1455: HValue* new_elements =
nit: Should fit into one line now.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode711
src/hydrogen.h:711: // Finished the current basic block and boolean for
nit: s/Finished/Finishes/ and missing "materialize" here.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode884
src/hydrogen.h:884: class IfContinuation {
Could I convince you to rename this class to "HContinuation" instead?

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode892
src/hydrogen.h:892: ASSERT(!continuation_captured_);
Can we also add an assert here that at least one of the blocks is non-NULL?

ASSERT(true_branch != NULL || false_branch != NULL);

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode1044
src/hydrogen.h:1044: IfBuilder(HGraphBuilder* builder,
nit: Move this in front of the destructor.

[danno, 2013-04-18 10:44:04]

Addressed feedback, landing.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.cc#newcode661
src/hydrogen.cc:661: builder_->CreateBasicBlock(env->Copy());
On 2013/04/18 10:30:21, Michael Starzinger wrote:
> nit: Should fit into one line now. Also three more occurrences below.

Done.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.cc#newcode1455
src/hydrogen.cc:1455: HValue* new_elements =
On 2013/04/18 10:30:21, Michael Starzinger wrote:
> nit: Should fit into one line now.

Done.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode711
src/hydrogen.h:711: // Finished the current basic block and boolean for
On 2013/04/18 10:30:21, Michael Starzinger wrote:
> nit: s/Finished/Finishes/ and missing "materialize" here.

Done.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode884
src/hydrogen.h:884: class IfContinuation {
On 2013/04/18 10:30:21, Michael Starzinger wrote:
> Could I convince you to rename this class to "HContinuation" instead?

Done.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode892
src/hydrogen.h:892: ASSERT(!continuation_captured_);
On 2013/04/18 10:30:21, Michael Starzinger wrote:
> Can we also add an assert here that at least one of the blocks is non-NULL?
> 
> ASSERT(true_branch != NULL || false_branch != NULL);

Done.

https://codereview.chromium.org/14344004/diff/1007/src/hydrogen.h#newcode1044
src/hydrogen.h:1044: IfBuilder(HGraphBuilder* builder,
On 2013/04/18 10:30:21, Michael Starzinger wrote:
> nit: Move this in front of the destructor.

Done.

[danno, 2013-04-18 10:51:34]

Committed patchset #3 manually as r14327 (presubmit successful).