From e8c0a459a999a58cd80719a6733cab69d2e1db8d Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Fri, 23 Jul 2010 11:20:59 +0000 Subject: [PATCH] Fix bug in r5123, Comparison(), by unusing results before unconditional jump to smi comparison JumpTarget. Review URL: http://codereview.chromium.org/3026019 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5126 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ia32/codegen-ia32.cc | 34 ++++++++++++++++++---------------- src/ia32/codegen-ia32.h | 6 ++---- src/x64/codegen-x64.cc | 32 +++++++++++++++++--------------- src/x64/codegen-x64.h | 6 ++---- 4 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index b236754..55317ee 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -1445,45 +1445,49 @@ bool CodeGenerator::FoldConstantSmis(Token::Value op, int left, int right) { } -void CodeGenerator::JumpIfBothSmiUsingTypeInfo(Register left, - Register right, - TypeInfo left_info, - TypeInfo right_info, +void CodeGenerator::JumpIfBothSmiUsingTypeInfo(Result* left, + Result* right, JumpTarget* both_smi) { + TypeInfo left_info = left->type_info(); + TypeInfo right_info = right->type_info(); if (left_info.IsDouble() || left_info.IsString() || right_info.IsDouble() || right_info.IsString()) { // We know that left and right are not both smi. Don't do any tests. return; } - if (left.is(right)) { + if (left->reg().is(right->reg())) { if (!left_info.IsSmi()) { - __ test(left, Immediate(kSmiTagMask)); + __ test(left->reg(), Immediate(kSmiTagMask)); both_smi->Branch(zero); } else { - if (FLAG_debug_code) __ AbortIfNotSmi(left); + if (FLAG_debug_code) __ AbortIfNotSmi(left->reg()); + left->Unuse(); + right->Unuse(); both_smi->Jump(); } } else if (!left_info.IsSmi()) { if (!right_info.IsSmi()) { Result temp = allocator_->Allocate(); ASSERT(temp.is_valid()); - __ mov(temp.reg(), left); - __ or_(temp.reg(), Operand(right)); + __ mov(temp.reg(), left->reg()); + __ or_(temp.reg(), Operand(right->reg())); __ test(temp.reg(), Immediate(kSmiTagMask)); temp.Unuse(); both_smi->Branch(zero); } else { - __ test(left, Immediate(kSmiTagMask)); + __ test(left->reg(), Immediate(kSmiTagMask)); both_smi->Branch(zero); } } else { - if (FLAG_debug_code) __ AbortIfNotSmi(left); + if (FLAG_debug_code) __ AbortIfNotSmi(left->reg()); if (!right_info.IsSmi()) { - __ test(right, Immediate(kSmiTagMask)); + __ test(right->reg(), Immediate(kSmiTagMask)); both_smi->Branch(zero); } else { - if (FLAG_debug_code) __ AbortIfNotSmi(right); + if (FLAG_debug_code) __ AbortIfNotSmi(right->reg()); + left->Unuse(); + right->Unuse(); both_smi->Jump(); } } @@ -2780,9 +2784,7 @@ void CodeGenerator::Comparison(AstNode* node, Register right_reg = right_side.reg(); // In-line check for comparing two smis. - JumpIfBothSmiUsingTypeInfo(left_side.reg(), right_side.reg(), - left_side.type_info(), right_side.type_info(), - &is_smi); + JumpIfBothSmiUsingTypeInfo(&left_side, &right_side, &is_smi); if (has_valid_frame()) { // Inline the equality check if both operands can't be a NaN. If both diff --git a/src/ia32/codegen-ia32.h b/src/ia32/codegen-ia32.h index 45b6b41..66014a0 100644 --- a/src/ia32/codegen-ia32.h +++ b/src/ia32/codegen-ia32.h @@ -524,10 +524,8 @@ class CodeGenerator: public AstVisitor { // advantage of TypeInfo to skip unneeded checks. // Allocates a temporary register, possibly spilling from the frame, // if it needs to check both left and right. - void JumpIfBothSmiUsingTypeInfo(Register left, - Register right, - TypeInfo left_info, - TypeInfo right_info, + void JumpIfBothSmiUsingTypeInfo(Result* left, + Result* right, JumpTarget* both_smi); // Emits code sequence that jumps to deferred code if the inputs diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index b76c58e..4ba9fae 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -1201,40 +1201,44 @@ bool CodeGenerator::FoldConstantSmis(Token::Value op, int left, int right) { } -void CodeGenerator::JumpIfBothSmiUsingTypeInfo(Register left, - Register right, - TypeInfo left_info, - TypeInfo right_info, +void CodeGenerator::JumpIfBothSmiUsingTypeInfo(Result* left, + Result* right, JumpTarget* both_smi) { + TypeInfo left_info = left->type_info(); + TypeInfo right_info = right->type_info(); if (left_info.IsDouble() || left_info.IsString() || right_info.IsDouble() || right_info.IsString()) { // We know that left and right are not both smi. Don't do any tests. return; } - if (left.is(right)) { + if (left->reg().is(right->reg())) { if (!left_info.IsSmi()) { - Condition is_smi = masm()->CheckSmi(left); + Condition is_smi = masm()->CheckSmi(left->reg()); both_smi->Branch(is_smi); } else { - if (FLAG_debug_code) __ AbortIfNotSmi(left); + if (FLAG_debug_code) __ AbortIfNotSmi(left->reg()); + left->Unuse(); + right->Unuse(); both_smi->Jump(); } } else if (!left_info.IsSmi()) { if (!right_info.IsSmi()) { - Condition is_smi = masm()->CheckBothSmi(left, right); + Condition is_smi = masm()->CheckBothSmi(left->reg(), right->reg()); both_smi->Branch(is_smi); } else { - Condition is_smi = masm()->CheckSmi(left); + Condition is_smi = masm()->CheckSmi(left->reg()); both_smi->Branch(is_smi); } } else { - if (FLAG_debug_code) __ AbortIfNotSmi(left); + if (FLAG_debug_code) __ AbortIfNotSmi(left->reg()); if (!right_info.IsSmi()) { - Condition is_smi = masm()->CheckSmi(right); + Condition is_smi = masm()->CheckSmi(right->reg()); both_smi->Branch(is_smi); } else { - if (FLAG_debug_code) __ AbortIfNotSmi(right); + if (FLAG_debug_code) __ AbortIfNotSmi(right->reg()); + left->Unuse(); + right->Unuse(); both_smi->Jump(); } } @@ -2283,9 +2287,7 @@ void CodeGenerator::Comparison(AstNode* node, Register right_reg = right_side.reg(); // In-line check for comparing two smis. - JumpIfBothSmiUsingTypeInfo(left_side.reg(), right_side.reg(), - left_side.type_info(), right_side.type_info(), - &is_smi); + JumpIfBothSmiUsingTypeInfo(&left_side, &right_side, &is_smi); if (has_valid_frame()) { // Inline the equality check if both operands can't be a NaN. If both diff --git a/src/x64/codegen-x64.h b/src/x64/codegen-x64.h index da6d08a..f694dde 100644 --- a/src/x64/codegen-x64.h +++ b/src/x64/codegen-x64.h @@ -495,10 +495,8 @@ class CodeGenerator: public AstVisitor { // Emits code sequence that jumps to a JumpTarget if the inputs // are both smis. Cannot be in MacroAssembler because it takes // advantage of TypeInfo to skip unneeded checks. - void JumpIfBothSmiUsingTypeInfo(Register left, - Register right, - TypeInfo left_info, - TypeInfo right_info, + void JumpIfBothSmiUsingTypeInfo(Result* left, + Result* right, JumpTarget* both_smi); // Emits code sequence that jumps to deferred code if the input -- 2.7.4