From a172a5e839cbd5e0a6340f16254025a58afbf40f Mon Sep 17 00:00:00 2001 From: "dslomov@chromium.org" Date: Wed, 3 Apr 2013 16:25:24 +0000 Subject: [PATCH] Remove (H|L)JSArrayLength instructions BUG= Review URL: https://codereview.chromium.org/12491023 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14127 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 6 ---- src/arm/lithium-arm.h | 14 -------- src/arm/lithium-codegen-arm.cc | 7 ---- src/code-stubs-hydrogen.cc | 5 ++- src/hydrogen-instructions.cc | 13 +++---- src/hydrogen-instructions.h | 69 +++++++++++++----------------------- src/hydrogen.cc | 21 +++++------ src/hydrogen.h | 2 -- src/ia32/lithium-codegen-ia32.cc | 7 ---- src/ia32/lithium-ia32.cc | 6 ---- src/ia32/lithium-ia32.h | 14 -------- src/mips/lithium-codegen-mips.cc | 7 ---- src/mips/lithium-mips.cc | 6 ---- src/mips/lithium-mips.h | 14 -------- src/x64/lithium-codegen-x64.cc | 7 ---- src/x64/lithium-x64.cc | 6 ---- src/x64/lithium-x64.h | 14 -------- test/mjsunit/array-non-smi-length.js | 46 ++++++++++++++++++++++++ 18 files changed, 85 insertions(+), 179 deletions(-) create mode 100644 test/mjsunit/array-non-smi-length.js diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index 7ef1bbe..f2b65ef 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -1740,12 +1740,6 @@ LInstruction* LChunkBuilder::DoClassOfTestAndBranch( } -LInstruction* LChunkBuilder::DoJSArrayLength(HJSArrayLength* instr) { - LOperand* array = UseRegisterAtStart(instr->value()); - return DefineAsRegister(new(zone()) LJSArrayLength(array)); -} - - LInstruction* LChunkBuilder::DoFixedArrayBaseLength( HFixedArrayBaseLength* instr) { LOperand* array = UseRegisterAtStart(instr->value()); diff --git a/src/arm/lithium-arm.h b/src/arm/lithium-arm.h index e393c0e..207faf4 100644 --- a/src/arm/lithium-arm.h +++ b/src/arm/lithium-arm.h @@ -120,7 +120,6 @@ class LCodeGen; V(IsStringAndBranch) \ V(IsSmiAndBranch) \ V(IsUndetectableAndBranch) \ - V(JSArrayLength) \ V(Label) \ V(LazyBailout) \ V(LoadContextSlot) \ @@ -1165,19 +1164,6 @@ class LCmpMapAndBranch: public LTemplateInstruction<0, 1, 1> { }; -class LJSArrayLength: public LTemplateInstruction<1, 1, 0> { - public: - explicit LJSArrayLength(LOperand* value) { - inputs_[0] = value; - } - - LOperand* value() { return inputs_[0]; } - - DECLARE_CONCRETE_INSTRUCTION(JSArrayLength, "js-array-length") - DECLARE_HYDROGEN_ACCESSOR(JSArrayLength) -}; - - class LFixedArrayBaseLength: public LTemplateInstruction<1, 1, 0> { public: explicit LFixedArrayBaseLength(LOperand* value) { diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 687f6d9..7bb3535 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -1918,13 +1918,6 @@ void LCodeGen::DoConstantT(LConstantT* instr) { } -void LCodeGen::DoJSArrayLength(LJSArrayLength* instr) { - Register result = ToRegister(instr->result()); - Register array = ToRegister(instr->value()); - __ ldr(result, FieldMemOperand(array, JSArray::kLengthOffset)); -} - - void LCodeGen::DoFixedArrayBaseLength(LFixedArrayBaseLength* instr) { Register result = ToRegister(instr->result()); Register array = ToRegister(instr->value()); diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 3420c48..ee903ae 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -287,9 +287,8 @@ HValue* CodeStubGraphBuilder::BuildCodeStub() { AddInstruction(new(zone) HTrapAllocationMemento(js_array)); HInstruction* array_length = - AddInstruction(new(zone) HJSArrayLength(js_array, - js_array, - HType::Smi())); + AddInstruction(HLoadNamedField::NewArrayLength( + zone, js_array, js_array, HType::Smi())); ElementsKind to_kind = casted_stub()->to_kind(); BuildNewSpaceArrayCheck(array_length, to_kind); diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 8e0d89e..edbffc2 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -1468,15 +1468,6 @@ void HChange::PrintDataTo(StringStream* stream) { } -void HJSArrayLength::PrintDataTo(StringStream* stream) { - value()->PrintNameTo(stream); - if (HasTypeCheck()) { - stream->Add(" "); - typecheck()->PrintNameTo(stream); - } -} - - HValue* HUnaryMathOperation::Canonicalize() { if (op() == kMathFloor) { // If the input is integer32 then we replace the floor instruction @@ -2416,6 +2407,10 @@ void HParameter::PrintDataTo(StringStream* stream) { void HLoadNamedField::PrintDataTo(StringStream* stream) { object()->PrintNameTo(stream); stream->Add(" @%d%s", offset(), is_in_object() ? "[in-object]" : ""); + if (HasTypeCheck()) { + stream->Add(" "); + typecheck()->PrintNameTo(stream); + } } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index b568d21..ad03687 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -134,7 +134,6 @@ class LChunkBuilder; V(IsStringAndBranch) \ V(IsSmiAndBranch) \ V(IsUndetectableAndBranch) \ - V(JSArrayLength) \ V(LeaveInlined) \ V(LoadContextSlot) \ V(LoadElements) \ @@ -2392,45 +2391,6 @@ class HCallRuntime: public HCall<1> { }; -class HJSArrayLength: public HTemplateInstruction<2> { - public: - HJSArrayLength(HValue* value, HValue* typecheck, - HType type = HType::Tagged()) { - set_type(type); - // The length of an array is stored as a tagged value in the array - // object. It is guaranteed to be 32 bit integer, but it can be - // represented as either a smi or heap number. - SetOperandAt(0, value); - SetOperandAt(1, typecheck != NULL ? typecheck : value); - set_representation(Representation::Tagged()); - SetFlag(kUseGVN); - SetGVNFlag(kDependsOnArrayLengths); - SetGVNFlag(kDependsOnMaps); - } - - virtual Representation RequiredInputRepresentation(int index) { - return Representation::Tagged(); - } - - virtual void PrintDataTo(StringStream* stream); - - HValue* value() { return OperandAt(0); } - HValue* typecheck() { - ASSERT(HasTypeCheck()); - return OperandAt(1); - } - bool HasTypeCheck() const { return OperandAt(0) != OperandAt(1); } - - DECLARE_CONCRETE_INSTRUCTION(JSArrayLength) - - protected: - virtual bool DataEquals(HValue* other_raw) { return true; } - - private: - virtual bool IsDeletable() const { return true; } -}; - - class HFixedArrayBaseLength: public HUnaryOperation { public: explicit HFixedArrayBaseLength(HValue* value) : HUnaryOperation(value) { @@ -5192,12 +5152,16 @@ class HStoreContextSlot: public HTemplateInstruction<2> { }; -class HLoadNamedField: public HUnaryOperation { +class HLoadNamedField: public HTemplateInstruction<2> { public: - HLoadNamedField(HValue* object, bool is_in_object, int offset) - : HUnaryOperation(object), - is_in_object_(is_in_object), + HLoadNamedField(HValue* object, bool is_in_object, int offset, + HValue* typecheck = NULL) + : is_in_object_(is_in_object), offset_(offset) { + ASSERT(object != NULL); + SetOperandAt(0, object); + SetOperandAt(1, typecheck != NULL ? typecheck : object); + set_representation(Representation::Tagged()); SetFlag(kUseGVN); SetGVNFlag(kDependsOnMaps); @@ -5208,7 +5172,24 @@ class HLoadNamedField: public HUnaryOperation { } } + static HLoadNamedField* NewArrayLength(Zone* zone, HValue* object, + HValue* typecheck, + HType type = HType::Tagged()) { + HLoadNamedField* result = new(zone) HLoadNamedField( + object, true, JSArray::kLengthOffset, typecheck); + result->set_type(type); + result->SetGVNFlag(kDependsOnArrayLengths); + result->ClearGVNFlag(kDependsOnInobjectFields); + return result; + } + HValue* object() { return OperandAt(0); } + HValue* typecheck() { + ASSERT(HasTypeCheck()); + return OperandAt(1); + } + + bool HasTypeCheck() const { return OperandAt(0) != OperandAt(1); } bool is_in_object() const { return is_in_object_; } int offset() const { return offset_; } diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 20973f0..abff7b7 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1165,8 +1165,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess( } HInstruction* length = NULL; if (is_js_array) { - length = AddInstruction(new(zone) HJSArrayLength(object, mapcheck, - HType::Smi())); + length = AddInstruction( + HLoadNamedField::NewArrayLength(zone, object, mapcheck, HType::Smi())); } else { length = AddInstruction(new(zone) HFixedArrayBaseLength(elements)); } @@ -1235,13 +1235,6 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess( } -HInstruction* HGraphBuilder::BuildFastArrayLengthLoad(HValue* object, - HValue* typecheck) { - Zone* zone = this->zone(); - return new (zone) HJSArrayLength(object, typecheck, HType::Smi()); -} - - HValue* HGraphBuilder::BuildAllocateElements(HValue* context, ElementsKind kind, HValue* capacity) { @@ -6429,7 +6422,8 @@ bool HOptimizedGraphBuilder::HandlePolymorphicArrayLengthLoad( AddInstruction(new(zone()) HCheckNonSmi(object)); HInstruction* typecheck = AddInstruction(HCheckInstanceType::NewIsJSArray(object, zone())); - HInstruction* instr = BuildFastArrayLengthLoad(object, typecheck); + HInstruction* instr = + HLoadNamedField::NewArrayLength(zone(), object, typecheck); instr->set_position(expr->position()); ast_context()->ReturnInstruction(instr, expr->id()); return true; @@ -7094,7 +7088,7 @@ HInstruction* HOptimizedGraphBuilder::BuildLoadNamedMonomorphic( if (name->Equals(isolate()->heap()->length_string())) { if (map->instance_type() == JS_ARRAY_TYPE) { AddCheckMapsWithTransitions(object, map); - return BuildFastArrayLengthLoad(object, NULL); + return HLoadNamedField::NewArrayLength(zone(), object, object); } } @@ -7387,8 +7381,9 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess( set_current_block(if_jsarray); HInstruction* length; - length = AddInstruction(new(zone()) HJSArrayLength(object, typecheck, - HType::Smi())); + length = AddInstruction( + HLoadNamedField::NewArrayLength(zone(), object, typecheck, + HType::Smi())); checked_key = AddBoundsCheck(key, length, ALLOW_SMI_KEY); access = AddInstruction(BuildFastElementAccess( elements, checked_key, val, elements_kind_branch, diff --git a/src/hydrogen.h b/src/hydrogen.h index 30ec141..ef3679e 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -939,8 +939,6 @@ class HGraphBuilder { KeyedAccessStoreMode store_mode, Representation checked_index_representation = Representation::None()); - HInstruction* BuildFastArrayLengthLoad(HValue* object, HValue* typecheck); - HInstruction* BuildStoreMap(HValue* object, HValue* map, BailoutId id); HInstruction* BuildStoreMap(HValue* object, Handle map, BailoutId id); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index e0e0254..4d23aef 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -1740,13 +1740,6 @@ void LCodeGen::DoConstantT(LConstantT* instr) { } -void LCodeGen::DoJSArrayLength(LJSArrayLength* instr) { - Register result = ToRegister(instr->result()); - Register array = ToRegister(instr->value()); - __ mov(result, FieldOperand(array, JSArray::kLengthOffset)); -} - - void LCodeGen::DoFixedArrayBaseLength( LFixedArrayBaseLength* instr) { Register result = ToRegister(instr->result()); diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 0ec5d71..102515a 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -1749,12 +1749,6 @@ LInstruction* LChunkBuilder::DoClassOfTestAndBranch( } -LInstruction* LChunkBuilder::DoJSArrayLength(HJSArrayLength* instr) { - LOperand* array = UseRegisterAtStart(instr->value()); - return DefineAsRegister(new(zone()) LJSArrayLength(array)); -} - - LInstruction* LChunkBuilder::DoFixedArrayBaseLength( HFixedArrayBaseLength* instr) { LOperand* array = UseRegisterAtStart(instr->value()); diff --git a/src/ia32/lithium-ia32.h b/src/ia32/lithium-ia32.h index 0e36474..1c490bb 100644 --- a/src/ia32/lithium-ia32.h +++ b/src/ia32/lithium-ia32.h @@ -114,7 +114,6 @@ class LCodeGen; V(IsStringAndBranch) \ V(IsSmiAndBranch) \ V(IsUndetectableAndBranch) \ - V(JSArrayLength) \ V(Label) \ V(LazyBailout) \ V(LoadContextSlot) \ @@ -1147,19 +1146,6 @@ class LCmpMapAndBranch: public LTemplateInstruction<0, 1, 0> { }; -class LJSArrayLength: public LTemplateInstruction<1, 1, 0> { - public: - explicit LJSArrayLength(LOperand* value) { - inputs_[0] = value; - } - - LOperand* value() { return inputs_[0]; } - - DECLARE_CONCRETE_INSTRUCTION(JSArrayLength, "js-array-length") - DECLARE_HYDROGEN_ACCESSOR(JSArrayLength) -}; - - class LFixedArrayBaseLength: public LTemplateInstruction<1, 1, 0> { public: explicit LFixedArrayBaseLength(LOperand* value) { diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index 615e356..b5d6c45 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -1492,13 +1492,6 @@ void LCodeGen::DoConstantT(LConstantT* instr) { } -void LCodeGen::DoJSArrayLength(LJSArrayLength* instr) { - Register result = ToRegister(instr->result()); - Register array = ToRegister(instr->value()); - __ lw(result, FieldMemOperand(array, JSArray::kLengthOffset)); -} - - void LCodeGen::DoFixedArrayBaseLength(LFixedArrayBaseLength* instr) { Register result = ToRegister(instr->result()); Register array = ToRegister(instr->value()); diff --git a/src/mips/lithium-mips.cc b/src/mips/lithium-mips.cc index 8848032..6f96d44 100644 --- a/src/mips/lithium-mips.cc +++ b/src/mips/lithium-mips.cc @@ -1585,12 +1585,6 @@ LInstruction* LChunkBuilder::DoClassOfTestAndBranch( } -LInstruction* LChunkBuilder::DoJSArrayLength(HJSArrayLength* instr) { - LOperand* array = UseRegisterAtStart(instr->value()); - return DefineAsRegister(new(zone()) LJSArrayLength(array)); -} - - LInstruction* LChunkBuilder::DoFixedArrayBaseLength( HFixedArrayBaseLength* instr) { LOperand* array = UseRegisterAtStart(instr->value()); diff --git a/src/mips/lithium-mips.h b/src/mips/lithium-mips.h index 004e364..ebed526 100644 --- a/src/mips/lithium-mips.h +++ b/src/mips/lithium-mips.h @@ -120,7 +120,6 @@ class LCodeGen; V(IsStringAndBranch) \ V(IsSmiAndBranch) \ V(IsUndetectableAndBranch) \ - V(JSArrayLength) \ V(Label) \ V(LazyBailout) \ V(LoadContextSlot) \ @@ -1110,19 +1109,6 @@ class LCmpMapAndBranch: public LTemplateInstruction<0, 1, 1> { }; -class LJSArrayLength: public LTemplateInstruction<1, 1, 0> { - public: - explicit LJSArrayLength(LOperand* value) { - inputs_[0] = value; - } - - LOperand* value() { return inputs_[0]; } - - DECLARE_CONCRETE_INSTRUCTION(JSArrayLength, "js-array-length") - DECLARE_HYDROGEN_ACCESSOR(JSArrayLength) -}; - - class LFixedArrayBaseLength: public LTemplateInstruction<1, 1, 0> { public: explicit LFixedArrayBaseLength(LOperand* value) { diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 380e60e..817a9d3 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -1574,13 +1574,6 @@ void LCodeGen::DoConstantT(LConstantT* instr) { } -void LCodeGen::DoJSArrayLength(LJSArrayLength* instr) { - Register result = ToRegister(instr->result()); - Register array = ToRegister(instr->value()); - __ movq(result, FieldOperand(array, JSArray::kLengthOffset)); -} - - void LCodeGen::DoFixedArrayBaseLength(LFixedArrayBaseLength* instr) { Register result = ToRegister(instr->result()); Register array = ToRegister(instr->value()); diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index 5f62400..9a3166e 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -1661,12 +1661,6 @@ LInstruction* LChunkBuilder::DoClassOfTestAndBranch( } -LInstruction* LChunkBuilder::DoJSArrayLength(HJSArrayLength* instr) { - LOperand* array = UseRegisterAtStart(instr->value()); - return DefineAsRegister(new(zone()) LJSArrayLength(array)); -} - - LInstruction* LChunkBuilder::DoFixedArrayBaseLength( HFixedArrayBaseLength* instr) { LOperand* array = UseRegisterAtStart(instr->value()); diff --git a/src/x64/lithium-x64.h b/src/x64/lithium-x64.h index c4f6d84..051f9a4 100644 --- a/src/x64/lithium-x64.h +++ b/src/x64/lithium-x64.h @@ -121,7 +121,6 @@ class LCodeGen; V(IsStringAndBranch) \ V(IsSmiAndBranch) \ V(IsUndetectableAndBranch) \ - V(JSArrayLength) \ V(Label) \ V(LazyBailout) \ V(LoadContextSlot) \ @@ -1109,19 +1108,6 @@ class LCmpMapAndBranch: public LTemplateInstruction<0, 1, 0> { }; -class LJSArrayLength: public LTemplateInstruction<1, 1, 0> { - public: - explicit LJSArrayLength(LOperand* value) { - inputs_[0] = value; - } - - LOperand* value() { return inputs_[0]; } - - DECLARE_CONCRETE_INSTRUCTION(JSArrayLength, "js-array-length") - DECLARE_HYDROGEN_ACCESSOR(JSArrayLength) -}; - - class LFixedArrayBaseLength: public LTemplateInstruction<1, 1, 0> { public: explicit LFixedArrayBaseLength(LOperand* value) { diff --git a/test/mjsunit/array-non-smi-length.js b/test/mjsunit/array-non-smi-length.js new file mode 100644 index 0000000..23a25ee --- /dev/null +++ b/test/mjsunit/array-non-smi-length.js @@ -0,0 +1,46 @@ +// 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 TestNonSmiArrayLength() { + function f(a) { + return a.length+1; + } + + var a = []; + a.length = 0xFFFF; + assertSame(0x10000, f(a)); + assertSame(0x10000, f(a)); + + %OptimizeFunctionOnNextCall(f); + a.length = 0xFFFFFFFF; + assertSame(0x100000000, f(a)); +} + +TestNonSmiArrayLength(); + -- 2.7.4