From: verwaest@chromium.org Date: Mon, 27 May 2013 17:33:14 +0000 (+0000) Subject: Fix the hole loading optimization. X-Git-Tag: upstream/4.7.83~14106 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=aa2444269bbb535be51b34d245f923b493abf617;p=platform%2Fupstream%2Fv8.git Fix the hole loading optimization. - Holes are only ever loaded as double or tagged. - Change to tagged has to deoptimize on undefined (no implicit conversions from double the hole NaN -> tagged undefined). BUG= R=jkummerow@chromium.org Review URL: https://chromiumcodereview.appspot.com/16099006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14829 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index aed6492..d15fe7a 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -4963,7 +4963,9 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, Label load_smi, heap_number, done; - if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) { + STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE > + NUMBER_CANDIDATE_IS_ANY_TAGGED); + if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) { // Smi check. __ UntagAndJumpIfSmi(scratch, input_reg, &load_smi); @@ -4974,14 +4976,20 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, if (deoptimize_on_undefined) { DeoptimizeIf(ne, env); } else { - Label heap_number; + Label heap_number, convert; __ b(eq, &heap_number); + // Convert undefined (and hole) to NaN. __ LoadRoot(ip, Heap::kUndefinedValueRootIndex); __ cmp(input_reg, Operand(ip)); + if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) { + __ b(eq, &convert); + __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); + __ cmp(input_reg, Operand(ip)); + } DeoptimizeIf(ne, env); - // Convert undefined to NaN. + __ bind(&convert); __ LoadRoot(ip, Heap::kNanValueRootIndex); __ sub(ip, ip, Operand(kHeapObjectTag)); __ vldr(result_reg, ip, HeapNumber::kValueOffset); @@ -5001,15 +5009,6 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, DeoptimizeIf(eq, env); } __ jmp(&done); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) { - __ SmiUntag(scratch, input_reg, SetCC); - DeoptimizeIf(cs, env); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) { - __ UntagAndJumpIfSmi(scratch, input_reg, &load_smi); - __ Vmov(result_reg, - FixedDoubleArray::hole_nan_as_double(), - no_reg); - __ b(&done); } else { __ SmiUntag(scratch, input_reg); ASSERT(mode == NUMBER_CANDIDATE_IS_SMI); @@ -5133,19 +5132,13 @@ void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) { NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED; HValue* value = instr->hydrogen()->value(); if (value->type().IsSmi()) { - if (value->IsLoadKeyed()) { - HLoadKeyed* load = HLoadKeyed::cast(value); - if (load->UsesMustHandleHole()) { - if (load->hole_mode() == ALLOW_RETURN_HOLE) { - mode = NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE; - } else { - mode = NUMBER_CANDIDATE_IS_SMI_OR_HOLE; - } - } else { - mode = NUMBER_CANDIDATE_IS_SMI; + mode = NUMBER_CANDIDATE_IS_SMI; + } else if (value->IsLoadKeyed()) { + HLoadKeyed* load = HLoadKeyed::cast(value); + if (load->UsesMustHandleHole()) { + if (load->hole_mode() == ALLOW_RETURN_HOLE) { + mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE; } - } else { - mode = NUMBER_CANDIDATE_IS_SMI; } } diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index f1346d5..e7e63d1 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -2720,11 +2720,14 @@ bool HLoadKeyed::UsesMustHandleHole() const { return false; } + // Holes are only returned as tagged values. + if (!representation().IsTagged()) { + return false; + } + for (HUseIterator it(uses()); !it.Done(); it.Advance()) { HValue* use = it.value(); - if (!use->IsChange() || !HChange::cast(use)->to().IsDouble()) { - return false; - } + if (!use->IsChange()) return false; } return true; diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 6fbfd15..5d4b90a 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -1718,7 +1718,9 @@ class HChange: public HUnaryOperation { ASSERT(!value->representation().Equals(to)); set_representation(to); SetFlag(kUseGVN); - if (deoptimize_on_undefined) SetFlag(kDeoptimizeOnUndefined); + if (deoptimize_on_undefined || to.IsTagged()) { + SetFlag(kDeoptimizeOnUndefined); + } if (is_truncating) SetFlag(kTruncatingToInt32); if (value->representation().IsSmi() || value->type().IsSmi()) { set_type(HType::Smi()); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index b3d12c0..b473f58 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -3988,8 +3988,8 @@ bool HGraph::Optimize(SmartArrayPointer* bailout_reason) { // This must happen after inferring representations. MergeRemovableSimulates(); - MarkDeoptimizeOnUndefined(); InsertRepresentationChanges(); + MarkDeoptimizeOnUndefined(); InitializeInferredTypes(); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 4a0895f..dddd1c6 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -5076,7 +5076,9 @@ void LCodeGen::EmitNumberUntagDNoSSE2(Register input_reg, NumberUntagDMode mode) { Label load_smi, done; - if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) { + STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE > + NUMBER_CANDIDATE_IS_ANY_TAGGED); + if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) { // Smi check. __ JumpIfSmi(input_reg, &load_smi, Label::kNear); @@ -5086,17 +5088,23 @@ void LCodeGen::EmitNumberUntagDNoSSE2(Register input_reg, if (deoptimize_on_undefined) { DeoptimizeIf(not_equal, env); } else { - Label heap_number; + Label heap_number, convert; __ j(equal, &heap_number, Label::kNear); + // Convert undefined (or hole) to NaN. __ cmp(input_reg, factory()->undefined_value()); + if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) { + __ j(equal, &convert, Label::kNear); + __ cmp(input_reg, factory()->the_hole_value()); + } DeoptimizeIf(not_equal, env); - // Convert undefined to NaN. + __ bind(&convert); ExternalReference nan = ExternalReference::address_of_canonical_non_hole_nan(); __ fld_d(Operand::StaticVariable(nan)); __ jmp(&done, Label::kNear); + __ bind(&heap_number); } // Heap number to x87 conversion. @@ -5117,16 +5125,6 @@ void LCodeGen::EmitNumberUntagDNoSSE2(Register input_reg, DeoptimizeIf(not_zero, env); } __ jmp(&done, Label::kNear); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) { - __ test(input_reg, Immediate(kSmiTagMask)); - DeoptimizeIf(not_equal, env); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) { - __ test(input_reg, Immediate(kSmiTagMask)); - __ j(zero, &load_smi); - ExternalReference hole_nan_reference = - ExternalReference::address_of_the_hole_nan(); - __ fld_d(Operand::StaticVariable(hole_nan_reference)); - __ jmp(&done, Label::kNear); } else { ASSERT(mode == NUMBER_CANDIDATE_IS_SMI); } @@ -5150,7 +5148,9 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, NumberUntagDMode mode) { Label load_smi, done; - if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) { + STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE > + NUMBER_CANDIDATE_IS_ANY_TAGGED); + if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) { // Smi check. __ JumpIfSmi(input_reg, &load_smi, Label::kNear); @@ -5160,13 +5160,18 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, if (deoptimize_on_undefined) { DeoptimizeIf(not_equal, env); } else { - Label heap_number; + Label heap_number, convert; __ j(equal, &heap_number, Label::kNear); + // Convert undefined (and hole) to NaN. __ cmp(input_reg, factory()->undefined_value()); + if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) { + __ j(equal, &convert, Label::kNear); + __ cmp(input_reg, factory()->the_hole_value()); + } DeoptimizeIf(not_equal, env); - // Convert undefined to NaN. + __ bind(&convert); ExternalReference nan = ExternalReference::address_of_canonical_non_hole_nan(); __ movdbl(result_reg, Operand::StaticVariable(nan)); @@ -5186,16 +5191,6 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, DeoptimizeIf(not_zero, env); } __ jmp(&done, Label::kNear); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) { - __ test(input_reg, Immediate(kSmiTagMask)); - DeoptimizeIf(not_equal, env); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) { - __ test(input_reg, Immediate(kSmiTagMask)); - __ j(zero, &load_smi); - ExternalReference hole_nan_reference = - ExternalReference::address_of_the_hole_nan(); - __ movdbl(result_reg, Operand::StaticVariable(hole_nan_reference)); - __ jmp(&done, Label::kNear); } else { ASSERT(mode == NUMBER_CANDIDATE_IS_SMI); } @@ -5495,20 +5490,12 @@ void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) { NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED; HValue* value = instr->hydrogen()->value(); - if (value->type().IsSmi()) { - if (value->IsLoadKeyed()) { - HLoadKeyed* load = HLoadKeyed::cast(value); - if (load->UsesMustHandleHole()) { - if (load->hole_mode() == ALLOW_RETURN_HOLE) { - mode = NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE; - } else { - mode = NUMBER_CANDIDATE_IS_SMI_OR_HOLE; - } - } else { - mode = NUMBER_CANDIDATE_IS_SMI; - } - } else { - mode = NUMBER_CANDIDATE_IS_SMI; + if (value->representation().IsSmi()) { + mode = NUMBER_CANDIDATE_IS_SMI; + } else if (value->IsLoadKeyed()) { + HLoadKeyed* load = HLoadKeyed::cast(value); + if (load->UsesMustHandleHole()) { + mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE; } } diff --git a/src/lithium.h b/src/lithium.h index 85df95a..170e5c8 100644 --- a/src/lithium.h +++ b/src/lithium.h @@ -769,9 +769,8 @@ int StackSlotOffset(int index); enum NumberUntagDMode { NUMBER_CANDIDATE_IS_SMI, - NUMBER_CANDIDATE_IS_SMI_OR_HOLE, - NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE, - NUMBER_CANDIDATE_IS_ANY_TAGGED + NUMBER_CANDIDATE_IS_ANY_TAGGED, + NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE }; diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index f9b6e81..b42e660 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -4663,7 +4663,9 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, NumberUntagDMode mode) { Label load_smi, done; - if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) { + STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE > + NUMBER_CANDIDATE_IS_ANY_TAGGED); + if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) { // Smi check. __ JumpIfSmi(input_reg, &load_smi, Label::kNear); @@ -4673,13 +4675,18 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, if (deoptimize_on_undefined) { DeoptimizeIf(not_equal, env); } else { - Label heap_number; + Label heap_number, convert; __ j(equal, &heap_number, Label::kNear); + // Convert undefined (and hole) to NaN. Compute NaN as 0/0. __ CompareRoot(input_reg, Heap::kUndefinedValueRootIndex); + if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) { + __ j(equal, &convert, Label::kNear); + __ CompareRoot(input_reg, Heap::kTheHoleValueRootIndex); + } DeoptimizeIf(not_equal, env); - // Convert undefined to NaN. Compute NaN as 0/0. + __ bind(&convert); __ xorps(result_reg, result_reg); __ divsd(result_reg, result_reg); __ jmp(&done, Label::kNear); @@ -4698,16 +4705,6 @@ void LCodeGen::EmitNumberUntagD(Register input_reg, DeoptimizeIf(not_zero, env); } __ jmp(&done, Label::kNear); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) { - __ testq(input_reg, Immediate(kSmiTagMask)); - DeoptimizeIf(not_equal, env); - } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) { - __ testq(input_reg, Immediate(kSmiTagMask)); - __ j(zero, &load_smi); - __ Set(kScratchRegister, BitCast( - FixedDoubleArray::hole_nan_as_double())); - __ movq(result_reg, kScratchRegister); - __ jmp(&done, Label::kNear); } else { ASSERT(mode == NUMBER_CANDIDATE_IS_SMI); } @@ -4802,19 +4799,11 @@ void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) { NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED; HValue* value = instr->hydrogen()->value(); if (value->type().IsSmi()) { - if (value->IsLoadKeyed()) { - HLoadKeyed* load = HLoadKeyed::cast(value); - if (load->UsesMustHandleHole()) { - if (load->hole_mode() == ALLOW_RETURN_HOLE) { - mode = NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE; - } else { - mode = NUMBER_CANDIDATE_IS_SMI_OR_HOLE; - } - } else { - mode = NUMBER_CANDIDATE_IS_SMI; - } - } else { - mode = NUMBER_CANDIDATE_IS_SMI; + mode = NUMBER_CANDIDATE_IS_SMI; + } else if (value->IsLoadKeyed()) { + HLoadKeyed* load = HLoadKeyed::cast(value); + if (load->UsesMustHandleHole()) { + mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE; } } diff --git a/test/mjsunit/regress/regress-convert-hole.js b/test/mjsunit/regress/regress-convert-hole.js new file mode 100644 index 0000000..1424221 --- /dev/null +++ b/test/mjsunit/regress/regress-convert-hole.js @@ -0,0 +1,52 @@ +// Copyright 2013 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Flags: --allow-natives-syntax + +function f(test, test2, a, i) { + var o = [0.5,1,,3]; + var d; + if (test) { + d = 1.5; + } else { + d = o[i]; + } + if (test2) { + d += 1; + } + a[i] = d; + return d; +} + +var a = [0, 0, 0, {}]; +f(true, false, a, 0); +f(true, true, a, 0); +f(false, false, a, 1); +f(false, true, a, 1); +%OptimizeFunctionOnNextCall(f); +f(false, false, a, 2); +assertEquals(undefined, a[2]);