From 8da222b010cb80d3c140d642b648dd25af53e893 Mon Sep 17 00:00:00 2001 From: "erik.corry@gmail.com" Date: Mon, 5 Jul 2010 11:03:16 +0000 Subject: [PATCH] Specialize GenericUnaryStub so that it knows whether it needs to take negative zero into account. Review URL: http://codereview.chromium.org/2850043 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5018 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/codegen-arm.cc | 42 ++++++++++++++++++++++++++++-------------- src/arm/full-codegen-arm.cc | 8 ++++++-- src/arm/macro-assembler-arm.cc | 6 +++--- src/arm/macro-assembler-arm.h | 2 +- src/codegen.cc | 14 ++++++++++---- src/codegen.h | 25 +++++++++++++++++++------ src/ia32/codegen-ia32.cc | 30 +++++++++++++++++------------- src/ia32/full-codegen-ia32.cc | 8 ++++++-- src/x64/codegen-x64.cc | 29 ++++++++++++++++++++++------- src/x64/full-codegen-x64.cc | 8 ++++++-- 10 files changed, 118 insertions(+), 54 deletions(-) diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index 4d18727..39fb5df 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -5423,9 +5423,13 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { frame_->EmitPush(r0); // r0 has result } else { - bool overwrite = + bool can_overwrite = (node->expression()->AsBinaryOperation() != NULL && node->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; + + bool no_negative_zero = node->expression()->no_negative_zero(); Load(node->expression()); switch (op) { case Token::NOT: @@ -5436,7 +5440,10 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { case Token::SUB: { frame_->PopToR0(); - GenericUnaryOpStub stub(Token::SUB, overwrite); + GenericUnaryOpStub stub( + Token::SUB, + overwrite, + no_negative_zero ? kIgnoreNegativeZero : kStrictNegativeZero); frame_->CallStub(&stub, 0); frame_->EmitPush(r0); // r0 has result break; @@ -8899,16 +8906,23 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { // Go slow case if the value of the expression is zero // to make sure that we switch between 0 and -0. - __ cmp(r0, Operand(0)); - __ b(eq, &slow); - - // The value of the expression is a smi that is not zero. Try - // optimistic subtraction '0 - value'. - __ rsb(r1, r0, Operand(0), SetCC); - __ b(vs, &slow); - - __ mov(r0, Operand(r1)); // Set r0 to result. - __ b(&done); + if (negative_zero_ == kStrictNegativeZero) { + // If we have to check for zero, then we can check for the max negative + // smi while we are at it. + __ bic(ip, r0, Operand(0x80000000), SetCC); + __ b(eq, &slow); + __ rsb(r0, r0, Operand(0)); + __ StubReturn(1); + } else { + // The value of the expression is a smi and 0 is OK for -0. Try + // optimistic subtraction '0 - value'. + __ rsb(r0, r0, Operand(0), SetCC); + __ StubReturn(1, vc); + // We don't have to reverse the optimistic neg since the only case + // where we fall through is the minimum negative Smi, which is the case + // where the neg leaves the register unchanged. + __ jmp(&slow); // Go slow on max negative Smi. + } __ bind(&try_float); __ ldr(r1, FieldMemOperand(r0, HeapObject::kMapOffset)); @@ -8916,7 +8930,7 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { __ cmp(r1, heap_number_map); __ b(ne, &slow); // r0 is a heap number. Get a new heap number in r1. - if (overwrite_) { + if (overwrite_ == UNARY_OVERWRITE) { __ ldr(r2, FieldMemOperand(r0, HeapNumber::kExponentOffset)); __ eor(r2, r2, Operand(HeapNumber::kSignMask)); // Flip sign. __ str(r2, FieldMemOperand(r0, HeapNumber::kExponentOffset)); @@ -8949,7 +8963,7 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { __ b(&done); __ bind(&try_float); - if (!overwrite_) { + if (!overwrite_ == UNARY_OVERWRITE) { // Allocate a fresh heap number, but don't overwrite r0 until // we're sure we can do it without going through the slow case // that needs the value in r0. diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 6732873..080cb83 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -2736,9 +2736,11 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { case Token::SUB: { Comment cmt(masm_, "[ UnaryOperation (SUB)"); - bool overwrite = + bool can_overwrite = (expr->expression()->AsBinaryOperation() != NULL && expr->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; GenericUnaryOpStub stub(Token::SUB, overwrite); // GenericUnaryOpStub expects the argument to be in the // accumulator register r0. @@ -2750,9 +2752,11 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { case Token::BIT_NOT: { Comment cmt(masm_, "[ UnaryOperation (BIT_NOT)"); - bool overwrite = + bool can_overwrite = (expr->expression()->AsBinaryOperation() != NULL && expr->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; GenericUnaryOpStub stub(Token::BIT_NOT, overwrite); // GenericUnaryOpStub expects the argument to be in the // accumulator register r0. diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index 81fc11e..2896cc9 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -1372,12 +1372,12 @@ void MacroAssembler::TailCallStub(CodeStub* stub, Condition cond) { } -void MacroAssembler::StubReturn(int argc) { +void MacroAssembler::StubReturn(int argc, Condition cond) { ASSERT(argc >= 1 && generating_stub()); if (argc > 1) { - add(sp, sp, Operand((argc - 1) * kPointerSize)); + add(sp, sp, Operand((argc - 1) * kPointerSize), LeaveCC, cond); } - Ret(); + Ret(cond); } diff --git a/src/arm/macro-assembler-arm.h b/src/arm/macro-assembler-arm.h index d57c565..f1f7de7 100644 --- a/src/arm/macro-assembler-arm.h +++ b/src/arm/macro-assembler-arm.h @@ -537,7 +537,7 @@ class MacroAssembler: public Assembler { void TailCallStub(CodeStub* stub, Condition cond = al); // Return from a code stub after popping its arguments. - void StubReturn(int argc); + void StubReturn(int argc, Condition cond = al); // Call a runtime routine. void CallRuntime(Runtime::Function* f, int num_arguments); diff --git a/src/codegen.cc b/src/codegen.cc index 686e173..8864c95 100644 --- a/src/codegen.cc +++ b/src/codegen.cc @@ -460,11 +460,17 @@ void CodeGenerator::CodeForSourcePosition(int pos) { const char* GenericUnaryOpStub::GetName() { switch (op_) { case Token::SUB: - return overwrite_ - ? "GenericUnaryOpStub_SUB_Overwrite" - : "GenericUnaryOpStub_SUB_Alloc"; + if (negative_zero_ == kStrictNegativeZero) { + return overwrite_ == UNARY_OVERWRITE + ? "GenericUnaryOpStub_SUB_Overwrite_Strict0" + : "GenericUnaryOpStub_SUB_Alloc_Strict0"; + } else { + return overwrite_ == UNARY_OVERWRITE + ? "GenericUnaryOpStub_SUB_Overwrite_Ignore0" + : "GenericUnaryOpStub_SUB_Alloc_Ignore0"; + } case Token::BIT_NOT: - return overwrite_ + return overwrite_ == UNARY_OVERWRITE ? "GenericUnaryOpStub_BIT_NOT_Overwrite" : "GenericUnaryOpStub_BIT_NOT_Alloc"; default: diff --git a/src/codegen.h b/src/codegen.h index 0576fbb..742bc3e 100644 --- a/src/codegen.h +++ b/src/codegen.h @@ -75,6 +75,7 @@ // Mode to overwrite BinaryExpression values. enum OverwriteMode { NO_OVERWRITE, OVERWRITE_LEFT, OVERWRITE_RIGHT }; +enum UnaryOverwriteMode { UNARY_OVERWRITE, UNARY_NO_OVERWRITE }; // Types of uncatchable exceptions. enum UncatchableExceptionType { OUT_OF_MEMORY, TERMINATION }; @@ -414,21 +415,33 @@ class InstanceofStub: public CodeStub { }; +enum NegativeZeroHandling { + kStrictNegativeZero, + kIgnoreNegativeZero +}; + + class GenericUnaryOpStub : public CodeStub { public: - GenericUnaryOpStub(Token::Value op, bool overwrite) - : op_(op), overwrite_(overwrite) { } + GenericUnaryOpStub(Token::Value op, + UnaryOverwriteMode overwrite, + NegativeZeroHandling negative_zero = kStrictNegativeZero) + : op_(op), overwrite_(overwrite), negative_zero_(negative_zero) { } private: Token::Value op_; - bool overwrite_; + UnaryOverwriteMode overwrite_; + NegativeZeroHandling negative_zero_; - class OverwriteField: public BitField {}; - class OpField: public BitField {}; + class OverwriteField: public BitField {}; + class NegativeZeroField: public BitField {}; + class OpField: public BitField {}; Major MajorKey() { return GenericUnaryOp; } int MinorKey() { - return OpField::encode(op_) | OverwriteField::encode(overwrite_); + return OpField::encode(op_) | + OverwriteField::encode(overwrite_) | + NegativeZeroField::encode(negative_zero_); } void Generate(MacroAssembler* masm); diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index fa09dd8..2e4635a 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -7583,9 +7583,12 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { frame_->Push(&value); } else { Load(node->expression()); - bool overwrite = + bool can_overwrite = (node->expression()->AsBinaryOperation() != NULL && node->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; + bool no_negative_zero = node->expression()->no_negative_zero(); switch (op) { case Token::NOT: case Token::DELETE: @@ -7594,7 +7597,10 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { break; case Token::SUB: { - GenericUnaryOpStub stub(Token::SUB, overwrite); + GenericUnaryOpStub stub( + Token::SUB, + overwrite, + no_negative_zero ? kIgnoreNegativeZero : kStrictNegativeZero); Result operand = frame_->Pop(); Result answer = frame_->CallStub(&stub, &operand); answer.set_type_info(TypeInfo::Number()); @@ -10934,10 +10940,12 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { __ test(eax, Immediate(kSmiTagMask)); __ j(not_zero, &try_float, not_taken); - // Go slow case if the value of the expression is zero - // to make sure that we switch between 0 and -0. - __ test(eax, Operand(eax)); - __ j(zero, &slow, not_taken); + if (negative_zero_ == kStrictNegativeZero) { + // Go slow case if the value of the expression is zero + // to make sure that we switch between 0 and -0. + __ test(eax, Operand(eax)); + __ j(zero, &slow, not_taken); + } // The value of the expression is a smi that is not zero. Try // optimistic subtraction '0 - value'. @@ -10945,11 +10953,7 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { __ mov(edx, Operand(eax)); __ Set(eax, Immediate(0)); __ sub(eax, Operand(edx)); - __ j(overflow, &undo, not_taken); - - // If result is a smi we are done. - __ test(eax, Immediate(kSmiTagMask)); - __ j(zero, &done, taken); + __ j(no_overflow, &done, taken); // Restore eax and go slow case. __ bind(&undo); @@ -10961,7 +10965,7 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { __ mov(edx, FieldOperand(eax, HeapObject::kMapOffset)); __ cmp(edx, Factory::heap_number_map()); __ j(not_equal, &slow); - if (overwrite_) { + if (overwrite_ == UNARY_OVERWRITE) { __ mov(edx, FieldOperand(eax, HeapNumber::kExponentOffset)); __ xor_(edx, HeapNumber::kSignMask); // Flip sign. __ mov(FieldOperand(eax, HeapNumber::kExponentOffset), edx); @@ -11002,7 +11006,7 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { // Try to store the result in a heap number. __ bind(&try_float); - if (!overwrite_) { + if (overwrite_ == UNARY_NO_OVERWRITE) { // Allocate a fresh heap number, but don't overwrite eax until // we're sure we can do it without going through the slow case // that needs the value in eax. diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 13173e2..2ca1105 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -2813,9 +2813,11 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { case Token::SUB: { Comment cmt(masm_, "[ UnaryOperation (SUB)"); - bool overwrite = + bool can_overwrite = (expr->expression()->AsBinaryOperation() != NULL && expr->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; GenericUnaryOpStub stub(Token::SUB, overwrite); // GenericUnaryOpStub expects the argument to be in the // accumulator register eax. @@ -2827,9 +2829,11 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { case Token::BIT_NOT: { Comment cmt(masm_, "[ UnaryOperation (BIT_NOT)"); - bool overwrite = + bool can_overwrite = (expr->expression()->AsBinaryOperation() != NULL && expr->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; GenericUnaryOpStub stub(Token::BIT_NOT, overwrite); // GenericUnaryOpStub expects the argument to be in the // accumulator register eax. diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 3b1aeae..f85ebc6 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -3373,9 +3373,12 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { } } else { - bool overwrite = + bool can_overwrite = (node->expression()->AsBinaryOperation() != NULL && node->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; + bool no_negative_zero = node->expression()->no_negative_zero(); Load(node->expression()); switch (op) { case Token::NOT: @@ -3385,7 +3388,10 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) { break; case Token::SUB: { - GenericUnaryOpStub stub(Token::SUB, overwrite); + GenericUnaryOpStub stub( + Token::SUB, + overwrite, + no_negative_zero ? kIgnoreNegativeZero : kStrictNegativeZero); Result operand = frame_->Pop(); Result answer = frame_->CallStub(&stub, &operand); answer.set_type_info(TypeInfo::Number()); @@ -8506,6 +8512,11 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { Label try_float; __ JumpIfNotSmi(rax, &try_float); + if (negative_zero_ == kIgnoreNegativeZero) { + __ SmiCompare(rax, Smi::FromInt(0)); + __ j(equal, &done); + } + // Enter runtime system if the value of the smi is zero // to make sure that we switch between 0 and -0. // Also enter it if the value of the smi is Smi::kMinValue. @@ -8513,10 +8524,14 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { // Either zero or Smi::kMinValue, neither of which become a smi when // negated. - __ SmiCompare(rax, Smi::FromInt(0)); - __ j(not_equal, &slow); - __ Move(rax, Factory::minus_zero_value()); - __ jmp(&done); + if (negative_zero_ == kStrictNegativeZero) { + __ SmiCompare(rax, Smi::FromInt(0)); + __ j(not_equal, &slow); + __ Move(rax, Factory::minus_zero_value()); + __ jmp(&done); + } else { + __ jmp(&slow); + } // Try floating point case. __ bind(&try_float); @@ -8529,7 +8544,7 @@ void GenericUnaryOpStub::Generate(MacroAssembler* masm) { __ shl(kScratchRegister, Immediate(63)); __ xor_(rdx, kScratchRegister); // Flip sign. // rdx is value to store. - if (overwrite_) { + if (overwrite_ == UNARY_OVERWRITE) { __ movq(FieldOperand(rax, HeapNumber::kValueOffset), rdx); } else { __ AllocateHeapNumber(rcx, rbx, &slow); diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index e3f74f6..c6be503 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -2807,9 +2807,11 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { case Token::SUB: { Comment cmt(masm_, "[ UnaryOperation (SUB)"); - bool overwrite = + bool can_overwrite = (expr->expression()->AsBinaryOperation() != NULL && expr->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; GenericUnaryOpStub stub(Token::SUB, overwrite); // GenericUnaryOpStub expects the argument to be in the // accumulator register rax. @@ -2821,9 +2823,11 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { case Token::BIT_NOT: { Comment cmt(masm_, "[ UnaryOperation (BIT_NOT)"); - bool overwrite = + bool can_overwrite = (expr->expression()->AsBinaryOperation() != NULL && expr->expression()->AsBinaryOperation()->ResultOverwriteAllowed()); + UnaryOverwriteMode overwrite = + can_overwrite ? UNARY_OVERWRITE : UNARY_NO_OVERWRITE; GenericUnaryOpStub stub(Token::BIT_NOT, overwrite); // GenericUnaryOpStub expects the argument to be in the // accumulator register rax. -- 2.7.4