From 0bb0d672b606bcb8d046399d6d9d0f34200d7ba3 Mon Sep 17 00:00:00 2001 From: "svenpanne@chromium.org" Date: Wed, 22 Jun 2011 08:28:35 +0000 Subject: [PATCH] Make ToBooleanStub more consistent across platforms. The declaration of the ToBoolean class moved to the platform-independent part and its implementations are now structurally very similar. This is just an intermediate cleanup step to add type recording at the call site. Note that the MIPS implementation has not really been touched, so it should continue to work, too. Review URL: http://codereview.chromium.org/7218012 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8359 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 73 +++++++++++++------------------- src/arm/code-stubs-arm.h | 13 ------ src/code-stubs.h | 12 ++++++ src/ia32/code-stubs-ia32.cc | 40 ++++++++--------- src/ia32/code-stubs-ia32.h | 12 ------ src/ia32/full-codegen-ia32.cc | 4 +- src/ia32/lithium-codegen-ia32.cc | 2 +- src/mips/code-stubs-mips.cc | 5 +-- src/mips/code-stubs-mips.h | 13 ------ src/x64/code-stubs-x64.cc | 40 ++++++++--------- src/x64/code-stubs-x64.h | 12 ------ src/x64/full-codegen-x64.cc | 4 +- src/x64/lithium-codegen-x64.cc | 2 +- 13 files changed, 90 insertions(+), 142 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 4bc33239b..b92c25a1f 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -1610,15 +1610,13 @@ void CompareStub::Generate(MacroAssembler* masm) { } -// This stub does not handle the inlined cases (Smis, Booleans, undefined). // The stub returns zero for false, and a non-zero value for true. void ToBooleanStub::Generate(MacroAssembler* masm) { // This stub uses VFP3 instructions. CpuFeatures::Scope scope(VFP3); - Label false_result; - Label not_heap_number; - Register scratch = r9.is(tos_) ? r7 : r9; + Label false_result, true_result, not_string; + const Register map = r9.is(tos_) ? r7 : r9; // undefined -> false __ LoadRoot(ip, Heap::kUndefinedValueRootIndex); @@ -1648,11 +1646,31 @@ void ToBooleanStub::Generate(MacroAssembler* masm) { __ cmp(tos_, ip); __ b(eq, &false_result); - // HeapNumber => false iff +0, -0, or NaN. - __ ldr(scratch, FieldMemOperand(tos_, HeapObject::kMapOffset)); - __ LoadRoot(ip, Heap::kHeapNumberMapRootIndex); - __ cmp(scratch, ip); - __ b(¬_heap_number, ne); + // Get the map of the heap object. + __ ldr(map, FieldMemOperand(tos_, HeapObject::kMapOffset)); + + // Undetectable -> false. + __ ldrb(ip, FieldMemOperand(map, Map::kBitFieldOffset)); + __ tst(ip, Operand(1 << Map::kIsUndetectable)); + __ b(&false_result, ne); + + // JavaScript object -> true. + __ CompareInstanceType(map, ip, FIRST_SPEC_OBJECT_TYPE); + // "tos_" is a register and contains a non-zero value. Hence we implicitly + // return true if the greater than condition is satisfied. + __ Ret(ge); + + // String value -> false iff empty. + __ CompareInstanceType(map, ip, FIRST_NONSTRING_TYPE); + __ b(¬_string, ge); + __ ldr(tos_, FieldMemOperand(tos_, String::kLengthOffset)); + // Return string length as boolean value, i.e. return false iff length is 0. + __ Ret(); + + __ bind(¬_string); + // HeapNumber -> false iff +0, -0, or NaN. + __ CompareRoot(map, Heap::kHeapNumberMapRootIndex); + __ b(&true_result, ne); __ vldr(d1, FieldMemOperand(tos_, HeapNumber::kValueOffset)); __ VFPCompareAndSetFlags(d1, 0.0); // "tos_" is a register, and contains a non zero value by default. @@ -1662,41 +1680,10 @@ void ToBooleanStub::Generate(MacroAssembler* masm) { __ mov(tos_, Operand(0, RelocInfo::NONE), LeaveCC, vs); // for FP_NAN __ Ret(); - __ bind(¬_heap_number); - - // It can be an undetectable object. - // Undetectable => false. - __ ldr(ip, FieldMemOperand(tos_, HeapObject::kMapOffset)); - __ ldrb(scratch, FieldMemOperand(ip, Map::kBitFieldOffset)); - __ and_(scratch, scratch, Operand(1 << Map::kIsUndetectable)); - __ cmp(scratch, Operand(1 << Map::kIsUndetectable)); - __ b(&false_result, eq); - - // JavaScript object => true. - __ ldr(scratch, FieldMemOperand(tos_, HeapObject::kMapOffset)); - __ ldrb(scratch, FieldMemOperand(scratch, Map::kInstanceTypeOffset)); - __ cmp(scratch, Operand(FIRST_SPEC_OBJECT_TYPE)); - // "tos_" is a register and contains a non-zero value. - // Hence we implicitly return true if the greater than - // condition is satisfied. - __ Ret(gt); - - // Check for string - __ ldr(scratch, FieldMemOperand(tos_, HeapObject::kMapOffset)); - __ ldrb(scratch, FieldMemOperand(scratch, Map::kInstanceTypeOffset)); - __ cmp(scratch, Operand(FIRST_NONSTRING_TYPE)); - // "tos_" is a register and contains a non-zero value. - // Hence we implicitly return true if the greater than - // condition is satisfied. - __ Ret(gt); - - // String value => false iff empty, i.e., length is zero - __ ldr(tos_, FieldMemOperand(tos_, String::kLengthOffset)); - // If length is zero, "tos_" contains zero ==> false. - // If length is not zero, "tos_" contains a non-zero value ==> true. + // Return 1/0 for true/false in tos_. + __ bind(&true_result); + __ mov(tos_, Operand(1, RelocInfo::NONE)); __ Ret(); - - // Return 0 in "tos_" for false . __ bind(&false_result); __ mov(tos_, Operand(0, RelocInfo::NONE)); __ Ret(); diff --git a/src/arm/code-stubs-arm.h b/src/arm/code-stubs-arm.h index befeac1ac..8e3e9dc00 100644 --- a/src/arm/code-stubs-arm.h +++ b/src/arm/code-stubs-arm.h @@ -58,19 +58,6 @@ class TranscendentalCacheStub: public CodeStub { }; -class ToBooleanStub: public CodeStub { - public: - explicit ToBooleanStub(Register tos) : tos_(tos) { } - - void Generate(MacroAssembler* masm); - - private: - Register tos_; - Major MajorKey() { return ToBoolean; } - int MinorKey() { return tos_.code(); } -}; - - class UnaryOpStub: public CodeStub { public: UnaryOpStub(Token::Value op, UnaryOverwriteMode mode) diff --git a/src/code-stubs.h b/src/code-stubs.h index 0ed37017b..3a756585e 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1001,6 +1001,18 @@ class KeyedStoreExternalArrayStub : public CodeStub { }; +class ToBooleanStub: public CodeStub { + public: + explicit ToBooleanStub(Register tos) : tos_(tos) { } + + void Generate(MacroAssembler* masm); + + private: + Register tos_; + Major MajorKey() { return ToBoolean; } + int MinorKey() { return tos_.code(); } +}; + } } // namespace v8::internal #endif // V8_CODE_STUBS_H_ diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 27dbe2109..8fd272b7e 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -237,55 +237,55 @@ void FastCloneShallowArrayStub::Generate(MacroAssembler* masm) { } -// NOTE: The stub does not handle the inlined cases (Smis, Booleans, undefined). +// The stub returns zero for false, and a non-zero value for true. void ToBooleanStub::Generate(MacroAssembler* masm) { Label false_result, true_result, not_string; - __ mov(eax, Operand(esp, 1 * kPointerSize)); Factory* factory = masm->isolate()->factory(); + const Register map = edx; + + __ mov(eax, Operand(esp, 1 * kPointerSize)); // undefined -> false __ cmp(eax, factory->undefined_value()); __ j(equal, &false_result); // Boolean -> its value - __ cmp(eax, factory->true_value()); - __ j(equal, &true_result); __ cmp(eax, factory->false_value()); __ j(equal, &false_result); + __ cmp(eax, factory->true_value()); + __ j(equal, &true_result); // Smis: 0 -> false, all other -> true __ test(eax, Operand(eax)); __ j(zero, &false_result); __ JumpIfSmi(eax, &true_result); - // 'null' => false. + // 'null' -> false. __ cmp(eax, factory->null_value()); __ j(equal, &false_result, Label::kNear); - // Get the map and type of the heap object. - __ mov(edx, FieldOperand(eax, HeapObject::kMapOffset)); - __ movzx_b(ecx, FieldOperand(edx, Map::kInstanceTypeOffset)); + // Get the map of the heap object. + __ mov(map, FieldOperand(eax, HeapObject::kMapOffset)); - // Undetectable => false. - __ test_b(FieldOperand(edx, Map::kBitFieldOffset), + // Undetectable -> false. + __ test_b(FieldOperand(map, Map::kBitFieldOffset), 1 << Map::kIsUndetectable); __ j(not_zero, &false_result, Label::kNear); - // JavaScript object => true. - __ CmpInstanceType(edx, FIRST_SPEC_OBJECT_TYPE); + // JavaScript object -> true. + __ CmpInstanceType(map, FIRST_SPEC_OBJECT_TYPE); __ j(above_equal, &true_result, Label::kNear); - // String value => false iff empty. - __ CmpInstanceType(edx, FIRST_NONSTRING_TYPE); + // String value -> false iff empty. + __ CmpInstanceType(map, FIRST_NONSTRING_TYPE); __ j(above_equal, ¬_string, Label::kNear); - STATIC_ASSERT(kSmiTag == 0); __ cmp(FieldOperand(eax, String::kLengthOffset), Immediate(0)); __ j(zero, &false_result, Label::kNear); __ jmp(&true_result, Label::kNear); __ bind(¬_string); - // HeapNumber => false iff +0, -0, or NaN. - __ cmp(edx, factory->heap_number_map()); + // HeapNumber -> false iff +0, -0, or NaN. + __ cmp(map, factory->heap_number_map()); __ j(not_equal, &true_result, Label::kNear); __ fldz(); __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); @@ -293,12 +293,12 @@ void ToBooleanStub::Generate(MacroAssembler* masm) { __ j(zero, &false_result, Label::kNear); // Fall through to |true_result|. - // Return 1/0 for true/false in eax. + // Return 1/0 for true/false in tos_. __ bind(&true_result); - __ mov(eax, 1); + __ mov(tos_, 1); __ ret(1 * kPointerSize); __ bind(&false_result); - __ mov(eax, 0); + __ mov(tos_, 0); __ ret(1 * kPointerSize); } diff --git a/src/ia32/code-stubs-ia32.h b/src/ia32/code-stubs-ia32.h index ead7761f6..d51549d54 100644 --- a/src/ia32/code-stubs-ia32.h +++ b/src/ia32/code-stubs-ia32.h @@ -60,18 +60,6 @@ class TranscendentalCacheStub: public CodeStub { }; -class ToBooleanStub: public CodeStub { - public: - ToBooleanStub() { } - - void Generate(MacroAssembler* masm); - - private: - Major MajorKey() { return ToBoolean; } - int MinorKey() { return 0; } -}; - - class UnaryOpStub: public CodeStub { public: UnaryOpStub(Token::Value op, UnaryOverwriteMode mode) diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index f46a30a5e..02fa2c29f 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -566,10 +566,10 @@ void FullCodeGenerator::DoTest(Expression* condition, Label* if_true, Label* if_false, Label* fall_through) { - ToBooleanStub stub; + ToBooleanStub stub(result_register()); __ push(result_register()); __ CallStub(&stub); - __ test(eax, Operand(eax)); + __ test(result_register(), Operand(result_register())); // The stub returns nonzero for true. Split(not_zero, if_true, if_false, fall_through); } diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index e0c84ee24..d427710d5 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -1411,7 +1411,7 @@ void LCodeGen::DoBranch(LBranch* instr) { // The conversion stub doesn't cause garbage collections so it's // safe to not record a safepoint after the call. __ bind(&call_stub); - ToBooleanStub stub; + ToBooleanStub stub(eax); __ pushad(); __ push(reg); __ CallStub(&stub); diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index 0592dbf96..1aa1838be 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -1718,7 +1718,6 @@ void CompareStub::Generate(MacroAssembler* masm) { } -// This stub does not handle the inlined cases (Smis, Booleans, undefined). // The stub returns zero for false, and a non-zero value for true. void ToBooleanStub::Generate(MacroAssembler* masm) { // This stub uses FPU instructions. @@ -1782,7 +1781,7 @@ void ToBooleanStub::Generate(MacroAssembler* masm) { // "tos_" is a register and contains a non-zero value. // Hence we implicitly return true if the greater than // condition is satisfied. - __ Ret(gt, scratch0, Operand(FIRST_SPEC_OBJECT_TYPE)); + __ Ret(ge, scratch0, Operand(FIRST_SPEC_OBJECT_TYPE)); // Check for string. __ lw(scratch0, FieldMemOperand(tos_, HeapObject::kMapOffset)); @@ -1790,7 +1789,7 @@ void ToBooleanStub::Generate(MacroAssembler* masm) { // "tos_" is a register and contains a non-zero value. // Hence we implicitly return true if the greater than // condition is satisfied. - __ Ret(gt, scratch0, Operand(FIRST_NONSTRING_TYPE)); + __ Ret(ge, scratch0, Operand(FIRST_NONSTRING_TYPE)); // String value => false iff empty, i.e., length is zero. __ lw(tos_, FieldMemOperand(tos_, String::kLengthOffset)); diff --git a/src/mips/code-stubs-mips.h b/src/mips/code-stubs-mips.h index c2bd22566..e2323c174 100644 --- a/src/mips/code-stubs-mips.h +++ b/src/mips/code-stubs-mips.h @@ -59,19 +59,6 @@ class TranscendentalCacheStub: public CodeStub { }; -class ToBooleanStub: public CodeStub { - public: - explicit ToBooleanStub(Register tos) : tos_(tos) { } - - void Generate(MacroAssembler* masm); - - private: - Register tos_; - Major MajorKey() { return ToBoolean; } - int MinorKey() { return tos_.code(); } -}; - - class UnaryOpStub: public CodeStub { public: UnaryOpStub(Token::Value op, UnaryOverwriteMode mode) diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 0c33ea33d..e8642a5b7 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -231,8 +231,11 @@ void FastCloneShallowArrayStub::Generate(MacroAssembler* masm) { } +// The stub returns zero for false, and a non-zero value for true. void ToBooleanStub::Generate(MacroAssembler* masm) { Label false_result, true_result, not_string; + const Register map = rdx; + __ movq(rax, Operand(rsp, 1 * kPointerSize)); // undefined -> false @@ -250,48 +253,45 @@ void ToBooleanStub::Generate(MacroAssembler* masm) { __ j(equal, &false_result); __ JumpIfSmi(rax, &true_result); - // 'null' => false. + // 'null' -> false. __ CompareRoot(rax, Heap::kNullValueRootIndex); __ j(equal, &false_result, Label::kNear); - // Get the map and type of the heap object. - // We don't use CmpObjectType because we manipulate the type field. - __ movq(rdx, FieldOperand(rax, HeapObject::kMapOffset)); - __ movzxbq(rcx, FieldOperand(rdx, Map::kInstanceTypeOffset)); + // Get the map of the heap object. + __ movq(map, FieldOperand(rax, HeapObject::kMapOffset)); - // Undetectable => false. - __ movzxbq(rbx, FieldOperand(rdx, Map::kBitFieldOffset)); - __ and_(rbx, Immediate(1 << Map::kIsUndetectable)); + // Undetectable -> false. + __ testb(FieldOperand(map, Map::kBitFieldOffset), + Immediate(1 << Map::kIsUndetectable)); __ j(not_zero, &false_result, Label::kNear); - // JavaScript object => true. - __ cmpq(rcx, Immediate(FIRST_SPEC_OBJECT_TYPE)); + // JavaScript object -> true. + __ CmpInstanceType(map, FIRST_SPEC_OBJECT_TYPE); __ j(above_equal, &true_result, Label::kNear); - // String value => false iff empty. - __ cmpq(rcx, Immediate(FIRST_NONSTRING_TYPE)); + // String value -> false iff empty. + __ CmpInstanceType(map, FIRST_NONSTRING_TYPE); __ j(above_equal, ¬_string, Label::kNear); - __ movq(rdx, FieldOperand(rax, String::kLengthOffset)); - __ SmiTest(rdx); + __ cmpq(FieldOperand(rax, String::kLengthOffset), Immediate(0)); __ j(zero, &false_result, Label::kNear); __ jmp(&true_result, Label::kNear); __ bind(¬_string); - __ CompareRoot(rdx, Heap::kHeapNumberMapRootIndex); - __ j(not_equal, &true_result, Label::kNear); - // HeapNumber => false iff +0, -0, or NaN. + // HeapNumber -> false iff +0, -0, or NaN. // These three cases set the zero flag when compared to zero using ucomisd. + __ CompareRoot(map, Heap::kHeapNumberMapRootIndex); + __ j(not_equal, &true_result, Label::kNear); __ xorps(xmm0, xmm0); __ ucomisd(xmm0, FieldOperand(rax, HeapNumber::kValueOffset)); __ j(zero, &false_result, Label::kNear); // Fall through to |true_result|. - // Return 1/0 for true/false in rax. + // Return 1/0 for true/false in tos_. __ bind(&true_result); - __ Set(rax, 1); + __ Set(tos_, 1); __ ret(1 * kPointerSize); __ bind(&false_result); - __ Set(rax, 0); + __ Set(tos_, 0); __ ret(1 * kPointerSize); } diff --git a/src/x64/code-stubs-x64.h b/src/x64/code-stubs-x64.h index 27744034a..a7ed91c50 100644 --- a/src/x64/code-stubs-x64.h +++ b/src/x64/code-stubs-x64.h @@ -59,18 +59,6 @@ class TranscendentalCacheStub: public CodeStub { }; -class ToBooleanStub: public CodeStub { - public: - ToBooleanStub() { } - - void Generate(MacroAssembler* masm); - - private: - Major MajorKey() { return ToBoolean; } - int MinorKey() { return 0; } -}; - - class UnaryOpStub: public CodeStub { public: UnaryOpStub(Token::Value op, UnaryOverwriteMode mode) diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index fba6cc204..adfc72a26 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -564,10 +564,10 @@ void FullCodeGenerator::DoTest(Expression* condition, Label* if_true, Label* if_false, Label* fall_through) { - ToBooleanStub stub; + ToBooleanStub stub(result_register()); __ push(result_register()); __ CallStub(&stub); - __ testq(rax, rax); + __ testq(result_register(), result_register()); // The stub returns nonzero for true. Split(not_zero, if_true, if_false, fall_through); } diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 6bfc21274..435079f3a 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -1413,7 +1413,7 @@ void LCodeGen::DoBranch(LBranch* instr) { // The conversion stub doesn't cause garbage collections so it's // safe to not record a safepoint after the call. __ bind(&call_stub); - ToBooleanStub stub; + ToBooleanStub stub(rax); __ Pushad(); __ push(reg); __ CallStub(&stub); -- 2.34.1