From 019e27d8dbba828ef69b1008ccebdcf7c3e05a77 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Mon, 31 Mar 2014 14:21:04 +0000 Subject: [PATCH] Reland and fix "Fix LoadFieldByIndex to take mutable heap-numbers into account."" BUG= R=hpayer@chromium.org Review URL: https://codereview.chromium.org/218663005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20358 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 4 +- src/arm/lithium-codegen-arm.cc | 49 +++++++++++++++++++ src/arm/lithium-codegen-arm.h | 4 ++ src/arm64/lithium-arm64.cc | 4 +- src/arm64/lithium-codegen-arm64.cc | 48 ++++++++++++++++++ src/arm64/lithium-codegen-arm64.h | 4 ++ src/handles.cc | 4 ++ src/hydrogen-instructions.h | 1 + src/ia32/lithium-codegen-ia32.cc | 46 +++++++++++++++++ src/ia32/lithium-codegen-ia32.h | 3 ++ src/ia32/lithium-ia32.cc | 4 +- src/objects.cc | 9 ++++ src/objects.h | 3 ++ src/runtime.cc | 13 +++++ src/runtime.h | 1 + src/x64/lithium-codegen-x64.cc | 45 +++++++++++++++++ src/x64/lithium-codegen-x64.h | 3 ++ src/x64/lithium-x64.cc | 4 +- test/mjsunit/fuzz-natives-part1.js | 3 ++ .../regress/regress-load-field-by-index.js | 22 +++++++++ 20 files changed, 270 insertions(+), 4 deletions(-) create mode 100644 test/mjsunit/regress/regress-load-field-by-index.js diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index c280999ca..ba44b509e 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -2539,7 +2539,9 @@ LInstruction* LChunkBuilder::DoCheckMapValue(HCheckMapValue* instr) { LInstruction* LChunkBuilder::DoLoadFieldByIndex(HLoadFieldByIndex* instr) { LOperand* object = UseRegister(instr->object()); LOperand* index = UseRegister(instr->index()); - return DefineAsRegister(new(zone()) LLoadFieldByIndex(object, index)); + LLoadFieldByIndex* load = new(zone()) LLoadFieldByIndex(object, index); + LInstruction* result = DefineSameAsFirst(load); + return AssignPointerMap(result); } } } // namespace v8::internal diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 9cf94eea8..ce7e91b93 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -5716,13 +5716,61 @@ void LCodeGen::DoCheckMapValue(LCheckMapValue* instr) { } +void LCodeGen::DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register result, + Register object, + Register index) { + PushSafepointRegistersScope scope(this, Safepoint::kWithRegisters); + __ Push(object); + __ Push(index); + __ mov(cp, Operand::Zero()); + __ CallRuntimeSaveDoubles(Runtime::kLoadMutableDouble); + RecordSafepointWithRegisters( + instr->pointer_map(), 2, Safepoint::kNoLazyDeopt); + __ StoreToSafepointRegisterSlot(r0, result); +} + + void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { + class DeferredLoadMutableDouble V8_FINAL : public LDeferredCode { + public: + DeferredLoadMutableDouble(LCodeGen* codegen, + LLoadFieldByIndex* instr, + Register result, + Register object, + Register index) + : LDeferredCode(codegen), + instr_(instr), + result_(result), + object_(object), + index_(index) { + } + virtual void Generate() V8_OVERRIDE { + codegen()->DoDeferredLoadMutableDouble(instr_, result_, object_, index_); + } + virtual LInstruction* instr() V8_OVERRIDE { return instr_; } + private: + LLoadFieldByIndex* instr_; + Register result_; + Register object_; + Register index_; + }; + Register object = ToRegister(instr->object()); Register index = ToRegister(instr->index()); Register result = ToRegister(instr->result()); Register scratch = scratch0(); + DeferredLoadMutableDouble* deferred; + deferred = new(zone()) DeferredLoadMutableDouble( + this, instr, result, object, index); + Label out_of_object, done; + + __ tst(index, Operand(Smi::FromInt(1))); + __ b(ne, deferred->entry()); + __ mov(index, Operand(index, ASR, 1)); + __ cmp(index, Operand::Zero()); __ b(lt, &out_of_object); @@ -5738,6 +5786,7 @@ void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { __ sub(scratch, result, Operand::PointerOffsetFromSmiKey(index)); __ ldr(result, FieldMemOperand(scratch, FixedArray::kHeaderSize - kPointerSize)); + __ bind(deferred->exit()); __ bind(&done); } diff --git a/src/arm/lithium-codegen-arm.h b/src/arm/lithium-codegen-arm.h index 21da500d0..38843f171 100644 --- a/src/arm/lithium-codegen-arm.h +++ b/src/arm/lithium-codegen-arm.h @@ -141,6 +141,10 @@ class LCodeGen: public LCodeGenBase { void DoDeferredInstanceOfKnownGlobal(LInstanceOfKnownGlobal* instr, Label* map_check); void DoDeferredInstanceMigration(LCheckMaps* instr, Register object); + void DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register result, + Register object, + Register index); // Parallel move support. void DoParallelMove(LParallelMove* move); diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc index 6dccc5ffa..4bed135bc 100644 --- a/src/arm64/lithium-arm64.cc +++ b/src/arm64/lithium-arm64.cc @@ -2562,7 +2562,9 @@ LInstruction* LChunkBuilder::DoCheckMapValue(HCheckMapValue* instr) { LInstruction* LChunkBuilder::DoLoadFieldByIndex(HLoadFieldByIndex* instr) { LOperand* object = UseRegisterAtStart(instr->object()); LOperand* index = UseRegister(instr->index()); - return DefineAsRegister(new(zone()) LLoadFieldByIndex(object, index)); + LLoadFieldByIndex* load = new(zone()) LLoadFieldByIndex(object, index); + LInstruction* result = DefineSameAsFirst(load); + return AssignPointerMap(result); } diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index abae91151..bea79ba58 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -5879,14 +5879,61 @@ void LCodeGen::DoWrapReceiver(LWrapReceiver* instr) { } +void LCodeGen::DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register result, + Register object, + Register index) { + PushSafepointRegistersScope scope(this, Safepoint::kWithRegisters); + __ Push(object); + __ Push(index); + __ Mov(cp, 0); + __ CallRuntimeSaveDoubles(Runtime::kLoadMutableDouble); + RecordSafepointWithRegisters( + instr->pointer_map(), 2, Safepoint::kNoLazyDeopt); + __ StoreToSafepointRegisterSlot(x0, result); +} + + void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { + class DeferredLoadMutableDouble V8_FINAL : public LDeferredCode { + public: + DeferredLoadMutableDouble(LCodeGen* codegen, + LLoadFieldByIndex* instr, + Register result, + Register object, + Register index) + : LDeferredCode(codegen), + instr_(instr), + result_(result), + object_(object), + index_(index) { + } + virtual void Generate() V8_OVERRIDE { + codegen()->DoDeferredLoadMutableDouble(instr_, result_, object_, index_); + } + virtual LInstruction* instr() V8_OVERRIDE { return instr_; } + private: + LLoadFieldByIndex* instr_; + Register result_; + Register object_; + Register index_; + }; Register object = ToRegister(instr->object()); Register index = ToRegister(instr->index()); Register result = ToRegister(instr->result()); __ AssertSmi(index); + DeferredLoadMutableDouble* deferred; + deferred = new(zone()) DeferredLoadMutableDouble( + this, instr, result, object, index); + Label out_of_object, done; + + __ TestAndBranchIfAnySet( + index, reinterpret_cast(Smi::FromInt(1)), deferred->entry()); + __ Mov(index, Operand(index, ASR, 1)); + __ Cmp(index, Smi::FromInt(0)); __ B(lt, &out_of_object); @@ -5902,6 +5949,7 @@ void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { __ Sub(result, result, Operand::UntagSmiAndScale(index, kPointerSizeLog2)); __ Ldr(result, FieldMemOperand(result, FixedArray::kHeaderSize - kPointerSize)); + __ Bind(deferred->exit()); __ Bind(&done); } diff --git a/src/arm64/lithium-codegen-arm64.h b/src/arm64/lithium-codegen-arm64.h index b1d8b70d5..e06f05cf4 100644 --- a/src/arm64/lithium-codegen-arm64.h +++ b/src/arm64/lithium-codegen-arm64.h @@ -149,6 +149,10 @@ class LCodeGen: public LCodeGenBase { void DoDeferredAllocate(LAllocate* instr); void DoDeferredInstanceOfKnownGlobal(LInstanceOfKnownGlobal* instr); void DoDeferredInstanceMigration(LCheckMaps* instr, Register object); + void DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register result, + Register object, + Register index); Operand ToOperand32(LOperand* op, IntegerSignedness signedness); diff --git a/src/handles.cc b/src/handles.cc index ea0f8aeba..edb657c53 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -682,6 +682,10 @@ Handle GetEnumPropertyKeys(Handle object, if (field_index >= map->inobject_properties()) { field_index = -(field_index - map->inobject_properties() + 1); } + field_index = field_index << 1; + if (details.representation().IsDouble()) { + field_index |= 1; + } indices->set(index, Smi::FromInt(field_index)); } } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 81ecb890b..f920a6529 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -7504,6 +7504,7 @@ class HLoadFieldByIndex V8_FINAL : public HTemplateInstruction<2> { HValue* index) { SetOperandAt(0, object); SetOperandAt(1, index); + SetChangesFlag(kNewSpacePromotion); set_representation(Representation::Tagged()); } diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index b3c06d677..4ffbe5377 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -6323,11 +6323,56 @@ void LCodeGen::DoCheckMapValue(LCheckMapValue* instr) { } +void LCodeGen::DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register object, + Register index) { + PushSafepointRegistersScope scope(this); + __ push(object); + __ push(index); + __ xor_(esi, esi); + __ CallRuntimeSaveDoubles(Runtime::kLoadMutableDouble); + RecordSafepointWithRegisters( + instr->pointer_map(), 2, Safepoint::kNoLazyDeopt); + __ StoreToSafepointRegisterSlot(object, eax); +} + + void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { + class DeferredLoadMutableDouble V8_FINAL : public LDeferredCode { + public: + DeferredLoadMutableDouble(LCodeGen* codegen, + LLoadFieldByIndex* instr, + Register object, + Register index, + const X87Stack& x87_stack) + : LDeferredCode(codegen, x87_stack), + instr_(instr), + object_(object), + index_(index) { + } + virtual void Generate() V8_OVERRIDE { + codegen()->DoDeferredLoadMutableDouble(instr_, object_, index_); + } + virtual LInstruction* instr() V8_OVERRIDE { return instr_; } + private: + LLoadFieldByIndex* instr_; + Register object_; + Register index_; + }; + Register object = ToRegister(instr->object()); Register index = ToRegister(instr->index()); + DeferredLoadMutableDouble* deferred; + deferred = new(zone()) DeferredLoadMutableDouble( + this, instr, object, index, x87_stack_); + Label out_of_object, done; + __ test(index, Immediate(Smi::FromInt(1))); + __ j(not_zero, deferred->entry()); + + __ sar(index, 1); + __ cmp(index, Immediate(0)); __ j(less, &out_of_object, Label::kNear); __ mov(object, FieldOperand(object, @@ -6344,6 +6389,7 @@ void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { index, times_half_pointer_size, FixedArray::kHeaderSize - kPointerSize)); + __ bind(deferred->exit()); __ bind(&done); } diff --git a/src/ia32/lithium-codegen-ia32.h b/src/ia32/lithium-codegen-ia32.h index 079595cba..b29f0f1a0 100644 --- a/src/ia32/lithium-codegen-ia32.h +++ b/src/ia32/lithium-codegen-ia32.h @@ -163,6 +163,9 @@ class LCodeGen: public LCodeGenBase { void DoDeferredInstanceOfKnownGlobal(LInstanceOfKnownGlobal* instr, Label* map_check); void DoDeferredInstanceMigration(LCheckMaps* instr, Register object); + void DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register object, + Register index); // Parallel move support. void DoParallelMove(LParallelMove* move); diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 8fa6dbdca..384a21c69 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -2702,7 +2702,9 @@ LInstruction* LChunkBuilder::DoCheckMapValue(HCheckMapValue* instr) { LInstruction* LChunkBuilder::DoLoadFieldByIndex(HLoadFieldByIndex* instr) { LOperand* object = UseRegister(instr->object()); LOperand* index = UseTempRegister(instr->index()); - return DefineSameAsFirst(new(zone()) LLoadFieldByIndex(object, index)); + LLoadFieldByIndex* load = new(zone()) LLoadFieldByIndex(object, index); + LInstruction* result = DefineSameAsFirst(load); + return AssignPointerMap(result); } diff --git a/src/objects.cc b/src/objects.cc index 9a6b26aaf..78d2b8a03 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -5641,6 +5641,15 @@ Handle JSObject::Copy(Handle object) { } +Handle JSObject::FastPropertyAt(Handle object, + Representation representation, + int index) { + Isolate* isolate = object->GetIsolate(); + CALL_HEAP_FUNCTION(isolate, + object->FastPropertyAt(representation, index), Object); +} + + template class JSObjectWalkVisitor { public: diff --git a/src/objects.h b/src/objects.h index 28b63f5e8..d9516e55b 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2618,6 +2618,9 @@ class JSObject: public JSReceiver { MUST_USE_RESULT inline MaybeObject* FastPropertyAt( Representation representation, int index); + static Handle FastPropertyAt(Handle object, + Representation representation, + int index); inline Object* RawFastPropertyAt(int index); inline void FastPropertyAtPut(int index, Object* value); diff --git a/src/runtime.cc b/src/runtime.cc index 849de3cb1..78f379513 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -14508,6 +14508,19 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_NotifyContextDisposed) { } +RUNTIME_FUNCTION(MaybeObject*, Runtime_LoadMutableDouble) { + HandleScope scope(isolate); + ASSERT(args.length() == 2); + CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0); + CONVERT_ARG_HANDLE_CHECKED(Smi, index, 1); + int idx = index->value() >> 1; + if (idx < 0) { + idx = -idx + object->map()->inobject_properties() - 1; + } + return *JSObject::FastPropertyAt(object, Representation::Double(), idx); +} + + RUNTIME_FUNCTION(MaybeObject*, Runtime_TryMigrateInstance) { HandleScope scope(isolate); ASSERT(args.length() == 1); diff --git a/src/runtime.h b/src/runtime.h index be0e545f4..f84355bfd 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -100,6 +100,7 @@ namespace internal { F(DebugCallbackSupportsStepping, 1, 1) \ F(DebugPrepareStepInIfStepping, 1, 1) \ F(FlattenString, 1, 1) \ + F(LoadMutableDouble, 2, 1) \ F(TryMigrateInstance, 1, 1) \ F(NotifyContextDisposed, 0, 1) \ \ diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 849e2d4ec..ac6d3700d 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -5564,11 +5564,55 @@ void LCodeGen::DoCheckMapValue(LCheckMapValue* instr) { } +void LCodeGen::DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register object, + Register index) { + PushSafepointRegistersScope scope(this); + __ Push(object); + __ Push(index); + __ xorp(rsi, rsi); + __ CallRuntimeSaveDoubles(Runtime::kLoadMutableDouble); + RecordSafepointWithRegisters( + instr->pointer_map(), 2, Safepoint::kNoLazyDeopt); + __ StoreToSafepointRegisterSlot(object, rax); +} + + void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { + class DeferredLoadMutableDouble V8_FINAL : public LDeferredCode { + public: + DeferredLoadMutableDouble(LCodeGen* codegen, + LLoadFieldByIndex* instr, + Register object, + Register index) + : LDeferredCode(codegen), + instr_(instr), + object_(object), + index_(index) { + } + virtual void Generate() V8_OVERRIDE { + codegen()->DoDeferredLoadMutableDouble(instr_, object_, index_); + } + virtual LInstruction* instr() V8_OVERRIDE { return instr_; } + private: + LLoadFieldByIndex* instr_; + Register object_; + Register index_; + }; + Register object = ToRegister(instr->object()); Register index = ToRegister(instr->index()); + DeferredLoadMutableDouble* deferred; + deferred = new(zone()) DeferredLoadMutableDouble(this, instr, object, index); + Label out_of_object, done; + __ Move(kScratchRegister, Smi::FromInt(1)); + __ testp(index, kScratchRegister); + __ j(not_zero, deferred->entry()); + + __ sarp(index, Immediate(1)); + __ SmiToInteger32(index, index); __ cmpl(index, Immediate(0)); __ j(less, &out_of_object, Label::kNear); @@ -5586,6 +5630,7 @@ void LCodeGen::DoLoadFieldByIndex(LLoadFieldByIndex* instr) { index, times_pointer_size, FixedArray::kHeaderSize - kPointerSize)); + __ bind(deferred->exit()); __ bind(&done); } diff --git a/src/x64/lithium-codegen-x64.h b/src/x64/lithium-codegen-x64.h index 37807ede0..23c63c816 100644 --- a/src/x64/lithium-codegen-x64.h +++ b/src/x64/lithium-codegen-x64.h @@ -116,6 +116,9 @@ class LCodeGen: public LCodeGenBase { void DoDeferredInstanceOfKnownGlobal(LInstanceOfKnownGlobal* instr, Label* map_check); void DoDeferredInstanceMigration(LCheckMaps* instr, Register object); + void DoDeferredLoadMutableDouble(LLoadFieldByIndex* instr, + Register object, + Register index); // Parallel move support. void DoParallelMove(LParallelMove* move); diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index 0dba33dc9..9d99e91d7 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -2589,7 +2589,9 @@ LInstruction* LChunkBuilder::DoCheckMapValue(HCheckMapValue* instr) { LInstruction* LChunkBuilder::DoLoadFieldByIndex(HLoadFieldByIndex* instr) { LOperand* object = UseRegister(instr->object()); LOperand* index = UseTempRegister(instr->index()); - return DefineSameAsFirst(new(zone()) LLoadFieldByIndex(object, index)); + LLoadFieldByIndex* load = new(zone()) LLoadFieldByIndex(object, index); + LInstruction* result = DefineSameAsFirst(load); + return AssignPointerMap(result); } diff --git a/test/mjsunit/fuzz-natives-part1.js b/test/mjsunit/fuzz-natives-part1.js index 0bd0dc1ab..2059bb0a6 100644 --- a/test/mjsunit/fuzz-natives-part1.js +++ b/test/mjsunit/fuzz-natives-part1.js @@ -182,6 +182,9 @@ var knownProblems = { // Only applicable to TypedArrays. "_TypedArrayInitialize": true, + // Only applicable to loading mutable doubles. + "LoadMutableDouble": true, + // Only applicable to generators. "_GeneratorNext": true, "_GeneratorThrow": true, diff --git a/test/mjsunit/regress/regress-load-field-by-index.js b/test/mjsunit/regress/regress-load-field-by-index.js new file mode 100644 index 000000000..c572c1ee3 --- /dev/null +++ b/test/mjsunit/regress/regress-load-field-by-index.js @@ -0,0 +1,22 @@ +// Copyright 2014 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +var o = {a:1.5, b:{}}; + +function f(o) { + var result = []; + for (var k in o) { + result[result.length] = o[k]; + } + return result; +} + +f(o); +f(o); +%OptimizeFunctionOnNextCall(f); +var array = f(o); +o.a = 1.7; +assertEquals(1.5, array[0]); -- 2.34.1