From 2e555d28875c00a5e203108ce1c615cf6a4c87ba Mon Sep 17 00:00:00 2001 From: "m.m.capewell@googlemail.com" Date: Tue, 18 Mar 2014 15:01:55 +0000 Subject: [PATCH] A64: ElementsKind TODOs Replace LoadElementsKind with LoadElementsKindFromMap, remove unneeded TODO in DoStringCharFromCode, improve constraints for DoCheckValue and improve code for ElementsKind checking in StoreArrayLiteralElementStub. BUG= R=ulan@chromium.org Review URL: https://codereview.chromium.org/202853005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20039 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/a64/code-stubs-a64.cc | 32 ++++++++++++++++++-------------- src/a64/lithium-a64.cc | 11 +++-------- src/a64/lithium-a64.h | 6 ++---- src/a64/lithium-codegen-a64.cc | 3 ++- src/a64/macro-assembler-a64.cc | 17 ++--------------- src/a64/macro-assembler-a64.h | 16 ++++------------ 6 files changed, 31 insertions(+), 54 deletions(-) diff --git a/src/a64/code-stubs-a64.cc b/src/a64/code-stubs-a64.cc index 7542391..adbe182 100644 --- a/src/a64/code-stubs-a64.cc +++ b/src/a64/code-stubs-a64.cc @@ -4791,12 +4791,6 @@ void RecordWriteStub::Generate(MacroAssembler* masm) { void StoreArrayLiteralElementStub::Generate(MacroAssembler* masm) { - // TODO(all): Possible optimisations in this function: - // 1. Merge CheckFastElements and CheckFastSmiElements, so that the map - // bitfield is loaded only once. - // 2. Refactor the Ldr/Add sequence at the start of fast_elements and - // smi_element. - // x0 value element value to store // x3 index_smi element index as smi // sp[0] array_index_smi array literal index in function as smi @@ -4812,9 +4806,23 @@ void StoreArrayLiteralElementStub::Generate(MacroAssembler* masm) { __ Ldr(array_map, FieldMemOperand(array, JSObject::kMapOffset)); Label double_elements, smi_element, fast_elements, slow_elements; - __ CheckFastElements(array_map, x10, &double_elements); + Register bitfield2 = x10; + __ Ldrb(bitfield2, FieldMemOperand(array_map, Map::kBitField2Offset)); + + // Jump if array's ElementsKind is not FAST*_SMI_ELEMENTS, FAST_ELEMENTS or + // FAST_HOLEY_ELEMENTS. + STATIC_ASSERT(FAST_SMI_ELEMENTS == 0); + STATIC_ASSERT(FAST_HOLEY_SMI_ELEMENTS == 1); + STATIC_ASSERT(FAST_ELEMENTS == 2); + STATIC_ASSERT(FAST_HOLEY_ELEMENTS == 3); + __ Cmp(bitfield2, Map::kMaximumBitField2FastHoleyElementValue); + __ B(hi, &double_elements); + __ JumpIfSmi(value, &smi_element); - __ CheckFastSmiElements(array_map, x10, &fast_elements); + + // Jump if array's ElementsKind is not FAST_ELEMENTS or FAST_HOLEY_ELEMENTS. + __ Tbnz(bitfield2, MaskToBit(FAST_ELEMENTS << Map::kElementsKindShift), + &fast_elements); // Store into the array literal requires an elements transition. Call into // the runtime. @@ -5540,12 +5548,8 @@ void InternalArrayConstructorStub::Generate(MacroAssembler* masm) { __ Ldr(x10, FieldMemOperand(constructor, JSFunction::kPrototypeOrInitialMapOffset)); - // TODO(jbramley): Add a helper function to read elements kind from an - // existing map. - // Load the map's "bit field 2" into result. - __ Ldr(kind, FieldMemOperand(x10, Map::kBitField2Offset)); - // Retrieve elements_kind from bit field 2. - __ Ubfx(kind, kind, Map::kElementsKindShift, Map::kElementsKindBitCount); + // Retrieve elements_kind from map. + __ LoadElementsKindFromMap(kind, x10); if (FLAG_debug_code) { Label done; diff --git a/src/a64/lithium-a64.cc b/src/a64/lithium-a64.cc index cc785dd..1c3b24e 100644 --- a/src/a64/lithium-a64.cc +++ b/src/a64/lithium-a64.cc @@ -1176,12 +1176,8 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) { LInstruction* LChunkBuilder::DoCheckValue(HCheckValue* instr) { - // We only need a temp register if the target is in new space, but we can't - // dereference the handle to test that here. - // TODO(all): Check these constraints. The temp register is not always used. - LOperand* value = UseRegister(instr->value()); - LOperand* temp = TempRegister(); - return AssignEnvironment(new(zone()) LCheckValue(value, temp)); + LOperand* value = UseRegisterAtStart(instr->value()); + return AssignEnvironment(new(zone()) LCheckValue(value)); } @@ -2292,7 +2288,6 @@ LInstruction* LChunkBuilder::DoStringCharCodeAt(HStringCharCodeAt* instr) { LInstruction* LChunkBuilder::DoStringCharFromCode(HStringCharFromCode* instr) { - // TODO(all) use at start and remove assert in codegen LOperand* char_code = UseRegister(instr->value()); LOperand* context = UseAny(instr->context()); LStringCharFromCode* result = @@ -2320,7 +2315,7 @@ LInstruction* LChunkBuilder::DoSub(HSub* instr) { ASSERT(instr->right()->representation().Equals(instr->representation())); LOperand *left; if (instr->left()->IsConstant() && - (HConstant::cast(instr->left())->Integer32Value() == 0)) { + (HConstant::cast(instr->left())->Integer32Value() == 0)) { left = UseConstant(instr->left()); } else { left = UseRegisterAtStart(instr->left()); diff --git a/src/a64/lithium-a64.h b/src/a64/lithium-a64.h index 7f96df1..a100da8 100644 --- a/src/a64/lithium-a64.h +++ b/src/a64/lithium-a64.h @@ -965,15 +965,13 @@ class LCheckSmi V8_FINAL : public LTemplateInstruction<1, 1, 0> { }; -class LCheckValue V8_FINAL : public LTemplateInstruction<0, 1, 1> { +class LCheckValue V8_FINAL : public LTemplateInstruction<0, 1, 0> { public: - LCheckValue(LOperand* value, LOperand* temp) { + explicit LCheckValue(LOperand* value) { inputs_[0] = value; - temps_[0] = temp; } LOperand* value() { return inputs_[0]; } - LOperand* temp() { return temps_[0]; } DECLARE_CONCRETE_INSTRUCTION(CheckValue, "check-value") DECLARE_HYDROGEN_ACCESSOR(CheckValue) diff --git a/src/a64/lithium-codegen-a64.cc b/src/a64/lithium-codegen-a64.cc index f39addb..18e353b 100644 --- a/src/a64/lithium-codegen-a64.cc +++ b/src/a64/lithium-codegen-a64.cc @@ -2538,7 +2538,8 @@ void LCodeGen::DoCheckValue(LCheckValue* instr) { Handle object = instr->hydrogen()->object().handle(); AllowDeferredHandleDereference smi_check; if (isolate()->heap()->InNewSpace(*object)) { - Register temp = ToRegister(instr->temp()); + UseScratchRegisterScope temps(masm()); + Register temp = temps.AcquireX(); Handle cell = isolate()->factory()->NewCell(object); __ Mov(temp, Operand(Handle(cell))); __ Ldr(temp, FieldMemOperand(temp, Cell::kValueOffset)); diff --git a/src/a64/macro-assembler-a64.cc b/src/a64/macro-assembler-a64.cc index 953f0ef..8a9bd61 100644 --- a/src/a64/macro-assembler-a64.cc +++ b/src/a64/macro-assembler-a64.cc @@ -3678,11 +3678,9 @@ void MacroAssembler::TestMapBitfield(Register object, uint64_t mask) { } -void MacroAssembler::LoadElementsKind(Register result, Register object) { - // Load map. - __ Ldr(result, FieldMemOperand(object, HeapObject::kMapOffset)); +void MacroAssembler::LoadElementsKindFromMap(Register result, Register map) { // Load the map's "bit field 2". - __ Ldrb(result, FieldMemOperand(result, Map::kBitField2Offset)); + __ Ldrb(result, FieldMemOperand(map, Map::kBitField2Offset)); // Retrieve elements_kind from bit field 2. __ Ubfx(result, result, Map::kElementsKindShift, Map::kElementsKindBitCount); } @@ -3840,17 +3838,6 @@ void MacroAssembler::CheckFastObjectElements(Register map, } -void MacroAssembler::CheckFastSmiElements(Register map, - Register scratch, - Label* fail) { - STATIC_ASSERT(FAST_SMI_ELEMENTS == 0); - STATIC_ASSERT(FAST_HOLEY_SMI_ELEMENTS == 1); - Ldrb(scratch, FieldMemOperand(map, Map::kBitField2Offset)); - Cmp(scratch, Map::kMaximumBitField2FastHoleySmiElementValue); - B(hi, fail); -} - - // Note: The ARM version of this clobbers elements_reg, but this version does // not. Some uses of this in A64 assume that elements_reg will be preserved. void MacroAssembler::StoreNumberToDoubleElements(Register value_reg, diff --git a/src/a64/macro-assembler-a64.h b/src/a64/macro-assembler-a64.h index 39d8957..c616dd8 100644 --- a/src/a64/macro-assembler-a64.h +++ b/src/a64/macro-assembler-a64.h @@ -1469,9 +1469,9 @@ class MacroAssembler : public Assembler { // flags. The object register is preserved. void TestMapBitfield(Register object, uint64_t mask); - // Load the elements kind field of an object, and return it in the result + // Load the elements kind field from a map, and return it in the result // register. - void LoadElementsKind(Register result, Register object); + void LoadElementsKindFromMap(Register result, Register map); // Compare the object in a register to a value from the root list. void CompareRoot(const Register& obj, Heap::RootListIndex index); @@ -1535,19 +1535,11 @@ class MacroAssembler : public Assembler { // Check if a map for a JSObject indicates that the object has fast elements. // Jump to the specified label if it does not. - void CheckFastElements(Register map, - Register scratch, - Label* fail); + void CheckFastElements(Register map, Register scratch, Label* fail); // Check if a map for a JSObject indicates that the object can have both smi // and HeapObject elements. Jump to the specified label if it does not. - void CheckFastObjectElements(Register map, - Register scratch, - Label* fail); - - // Check if a map for a JSObject indicates that the object has fast smi only - // elements. Jump to the specified label if it does not. - void CheckFastSmiElements(Register map, Register scratch, Label* fail); + void CheckFastObjectElements(Register map, Register scratch, Label* fail); // Check to see if number can be stored as a double in FastDoubleElements. // If it can, store it at the index specified by key_reg in the array, -- 2.7.4