Reviewers: Kevin Millikin, Description: Address review comments.
Only jump over 'else' part of a conditional if it is actually generated. Update a comment to more correctly reflect what is going on. Please review this at http://codereview.chromium.org/155272 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/arm/codegen-arm.cc Index: src/arm/codegen-arm.cc =================================================================== --- src/arm/codegen-arm.cc (revision 2401) +++ src/arm/codegen-arm.cc (working copy) @@ -2421,26 +2421,22 @@ Comment cmnt(masm_, "[ Conditional"); JumpTarget then; JumpTarget else_; - JumpTarget exit; LoadConditionAndSpill(node->condition(), NOT_INSIDE_TYPEOF, &then, &else_, true); - if (frame_ != NULL) { + if (has_valid_frame()) { Branch(false, &else_); } - if (frame_ != NULL || then.is_linked()) { + if (has_valid_frame() || then.is_linked()) { then.Bind(); LoadAndSpill(node->then_expression(), typeof_state()); } - if (frame_ != NULL) { - exit.Jump(); - } if (else_.is_linked()) { + JumpTarget exit; + if (has_valid_frame()) exit.Jump(); else_.Bind(); LoadAndSpill(node->else_expression(), typeof_state()); + if (exit.is_linked()) exit.Bind(); } - if (exit.is_linked()) { - exit.Bind(); - } ASSERT(frame_->height() == original_height + 1); } @@ -3615,9 +3611,8 @@ false_target(), true_target(), true); - // LoadConditionAndSpill might emit only unconditional jumps to - // the targets in which case cc_reg_ is not set. When that - // happens, don't attempt to negate the condition. + // LoadCondition may (and usually does) leave a test and branch to + // be emitted by the caller. In that case, negate the condition. if (has_cc()) cc_reg_ = NegateCondition(cc_reg_); } else if (op == Token::DELETE) { --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
