From 6a10a9af3b954e2e530bfd9fd1604a63f1f70735 Mon Sep 17 00:00:00 2001 From: rmcilroy Date: Tue, 13 Oct 2015 07:00:40 -0700 Subject: [PATCH] [Interpreter] Add array literal support. Adds array literal support to the interpreter. Currently constructed array elements don't have type feedback slots, so also adds support for generic keyed store operations. Adds the following bytecodes: - CreateArrayLiteral - KeyedStoreICGeneric BUG=v8:4280 LOG=N Review URL: https://codereview.chromium.org/1400753003 Cr-Commit-Position: refs/heads/master@{#31240} --- src/compiler/bytecode-graph-builder.cc | 12 ++ src/compiler/interpreter-assembler.cc | 27 +++++ src/compiler/interpreter-assembler.h | 5 + src/compiler/raw-machine-assembler.cc | 16 +++ src/compiler/raw-machine-assembler.h | 3 + src/interpreter/bytecode-array-builder.cc | 20 ++++ src/interpreter/bytecode-array-builder.h | 10 +- src/interpreter/bytecode-generator.cc | 41 ++++++- src/interpreter/bytecodes.h | 5 + src/interpreter/interpreter.cc | 41 +++++++ .../interpreter/test-bytecode-generator.cc | 107 ++++++++++++++++++ test/cctest/interpreter/test-interpreter.cc | 35 ++++++ .../bytecode-array-builder-unittest.cc | 6 +- 13 files changed, 324 insertions(+), 4 deletions(-) diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc index a0b2813014..e1ff362d1a 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/src/compiler/bytecode-graph-builder.cc @@ -333,6 +333,18 @@ void BytecodeGraphBuilder::VisitPopContext( } +void BytecodeGraphBuilder::VisitCreateArrayLiteral( + const interpreter::BytecodeArrayIterator& iterator) { + UNIMPLEMENTED(); +} + + +void BytecodeGraphBuilder::VisitKeyedStoreICGeneric( + const interpreter::BytecodeArrayIterator& iterator) { + UNIMPLEMENTED(); +} + + void BytecodeGraphBuilder::VisitCall( const interpreter::BytecodeArrayIterator& iterator) { UNIMPLEMENTED(); diff --git a/src/compiler/interpreter-assembler.cc b/src/compiler/interpreter-assembler.cc index 4ff8fecaf4..0ba6176f6b 100644 --- a/src/compiler/interpreter-assembler.cc +++ b/src/compiler/interpreter-assembler.cc @@ -111,6 +111,13 @@ Node* InterpreterAssembler::RegisterLocation(Node* reg_index) { } +Node* InterpreterAssembler::LoadRegister(interpreter::Register reg) { + return raw_assembler_->Load( + kMachAnyTagged, RegisterFileRawPointer(), + RegisterFrameOffset(Int32Constant(reg.ToOperand()))); +} + + Node* InterpreterAssembler::LoadRegister(Node* reg_index) { return raw_assembler_->Load(kMachAnyTagged, RegisterFileRawPointer(), RegisterFrameOffset(reg_index)); @@ -352,6 +359,18 @@ Node* InterpreterAssembler::CallIC(CallInterfaceDescriptor descriptor, } +Node* InterpreterAssembler::CallIC(CallInterfaceDescriptor descriptor, + Node* target, Node* arg1, Node* arg2, + Node* arg3) { + Node** args = zone()->NewArray(4); + args[0] = arg1; + args[1] = arg2; + args[2] = arg3; + args[3] = GetContext(); + return CallIC(descriptor, target, args); +} + + Node* InterpreterAssembler::CallIC(CallInterfaceDescriptor descriptor, Node* target, Node* arg1, Node* arg2, Node* arg3, Node* arg4) { @@ -418,6 +437,14 @@ Node* InterpreterAssembler::CallRuntime(Runtime::FunctionId function_id, } +Node* InterpreterAssembler::CallRuntime(Runtime::FunctionId function_id, + Node* arg1, Node* arg2, Node* arg3, + Node* arg4) { + return raw_assembler_->CallRuntime4(function_id, arg1, arg2, arg3, arg4, + GetContext()); +} + + void InterpreterAssembler::Return() { Node* exit_trampoline_code_object = HeapConstant(isolate()->builtins()->InterpreterExitTrampoline()); diff --git a/src/compiler/interpreter-assembler.h b/src/compiler/interpreter-assembler.h index 71296330c8..11f43ccf26 100644 --- a/src/compiler/interpreter-assembler.h +++ b/src/compiler/interpreter-assembler.h @@ -65,6 +65,7 @@ class InterpreterAssembler { void SetContext(Node* value); // Loads from and stores to the interpreter register file. + Node* LoadRegister(interpreter::Register reg); Node* LoadRegister(Node* reg_index); Node* StoreRegister(Node* value, Node* reg_index); @@ -105,6 +106,8 @@ class InterpreterAssembler { Node* CallJS(Node* function, Node* first_arg, Node* arg_count); // Call an IC code stub. + Node* CallIC(CallInterfaceDescriptor descriptor, Node* target, Node* arg1, + Node* arg2, Node* arg3); Node* CallIC(CallInterfaceDescriptor descriptor, Node* target, Node* arg1, Node* arg2, Node* arg3, Node* arg4); Node* CallIC(CallInterfaceDescriptor descriptor, Node* target, Node* arg1, @@ -114,6 +117,8 @@ class InterpreterAssembler { Node* CallRuntime(Node* function_id, Node* first_arg, Node* arg_count); Node* CallRuntime(Runtime::FunctionId function_id, Node* arg1); Node* CallRuntime(Runtime::FunctionId function_id, Node* arg1, Node* arg2); + Node* CallRuntime(Runtime::FunctionId function_id, Node* arg1, Node* arg2, + Node* arg3, Node* arg4); // Jump relative to the current bytecode by |jump_offset|. void Jump(Node* jump_offset); diff --git a/src/compiler/raw-machine-assembler.cc b/src/compiler/raw-machine-assembler.cc index 56a696b1b6..24e520d0bc 100644 --- a/src/compiler/raw-machine-assembler.cc +++ b/src/compiler/raw-machine-assembler.cc @@ -217,6 +217,22 @@ Node* RawMachineAssembler::CallRuntime2(Runtime::FunctionId function, } +Node* RawMachineAssembler::CallRuntime4(Runtime::FunctionId function, + Node* arg1, Node* arg2, Node* arg3, + Node* arg4, Node* context) { + CallDescriptor* descriptor = Linkage::GetRuntimeCallDescriptor( + zone(), function, 4, Operator::kNoProperties, false); + + Node* centry = HeapConstant(CEntryStub(isolate(), 1).GetCode()); + Node* ref = AddNode( + common()->ExternalConstant(ExternalReference(function, isolate()))); + Node* arity = Int32Constant(4); + + return AddNode(common()->Call(descriptor), centry, arg1, arg2, arg3, arg4, + ref, arity, context, graph()->start(), graph()->start()); +} + + Node* RawMachineAssembler::CallCFunction0(MachineType return_type, Node* function) { MachineSignature::Builder builder(zone(), 1, 0); diff --git a/src/compiler/raw-machine-assembler.h b/src/compiler/raw-machine-assembler.h index d084bd482b..c3db3be963 100644 --- a/src/compiler/raw-machine-assembler.h +++ b/src/compiler/raw-machine-assembler.h @@ -508,6 +508,9 @@ class RawMachineAssembler { // Call to a runtime function with two arguments. Node* CallRuntime2(Runtime::FunctionId function, Node* arg1, Node* arg2, Node* context); + // Call to a runtime function with four arguments. + Node* CallRuntime4(Runtime::FunctionId function, Node* arg1, Node* arg2, + Node* arg3, Node* arg4, Node* context); // Call to a C function with zero arguments. Node* CallCFunction0(MachineType return_type, Node* function); // Call to a C function with one parameter. diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc index cdc2442888..d99d0db756 100644 --- a/src/interpreter/bytecode-array-builder.cc +++ b/src/interpreter/bytecode-array-builder.cc @@ -343,6 +343,13 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::StoreKeyedProperty( } +BytecodeArrayBuilder& BytecodeArrayBuilder::GenericStoreKeyedProperty( + Register object, Register key) { + Output(Bytecode::kKeyedStoreICGeneric, object.ToOperand(), key.ToOperand()); + return *this; +} + + BytecodeArrayBuilder& BytecodeArrayBuilder::CreateClosure( PretenureFlag tenured) { DCHECK(FitsInImm8Operand(tenured)); @@ -351,6 +358,19 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CreateClosure( } +BytecodeArrayBuilder& BytecodeArrayBuilder::CreateArrayLiteral( + int literal_index, int flags) { + DCHECK(FitsInImm8Operand(flags)); // Flags should fit in 8 bytes. + if (FitsInIdx8Operand(literal_index)) { + Output(Bytecode::kCreateArrayLiteral, static_cast(literal_index), + static_cast(flags)); + } else { + UNIMPLEMENTED(); + } + return *this; +} + + BytecodeArrayBuilder& BytecodeArrayBuilder::PushContext(Register context) { Output(Bytecode::kPushContext, context.ToOperand()); return *this; diff --git a/src/interpreter/bytecode-array-builder.h b/src/interpreter/bytecode-array-builder.h index 26a8c99cb9..4bc709a47e 100644 --- a/src/interpreter/bytecode-array-builder.h +++ b/src/interpreter/bytecode-array-builder.h @@ -45,7 +45,7 @@ class BytecodeArrayBuilder { Register Parameter(int parameter_index) const; - // Constant loads to accumulator. + // Constant loads to the accumulator. BytecodeArrayBuilder& LoadLiteral(v8::internal::Smi* value); BytecodeArrayBuilder& LoadLiteral(Handle object); BytecodeArrayBuilder& LoadUndefined(); @@ -54,7 +54,7 @@ class BytecodeArrayBuilder { BytecodeArrayBuilder& LoadTrue(); BytecodeArrayBuilder& LoadFalse(); - // Global loads to accumulator and stores from accumulator. + // Global loads to accumulator and stores from the accumulator. BytecodeArrayBuilder& LoadGlobal(int slot_index); BytecodeArrayBuilder& StoreGlobal(int slot_index, LanguageMode language_mode); @@ -78,13 +78,19 @@ class BytecodeArrayBuilder { BytecodeArrayBuilder& StoreKeyedProperty(Register object, Register key, int feedback_slot, LanguageMode language_mode); + BytecodeArrayBuilder& GenericStoreKeyedProperty(Register object, + Register key); // Create a new closure for the SharedFunctionInfo in the accumulator. BytecodeArrayBuilder& CreateClosure(PretenureFlag tenured); + // Literals creation. Constant elements should be in the accumulator. + BytecodeArrayBuilder& CreateArrayLiteral(int literal_index, int flags); + // Push the context in accumulator as the new context, and store in register // |context|. BytecodeArrayBuilder& PushContext(Register context); + // Pop the current context and replace with |context|. BytecodeArrayBuilder& PopContext(Register context); diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 2eb235df04..cf20652a58 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -9,6 +9,7 @@ #include "src/compiler.h" #include "src/interpreter/control-flow-builders.h" #include "src/objects.h" +#include "src/parser.h" #include "src/scopes.h" #include "src/token.h" @@ -501,7 +502,45 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void BytecodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { - UNIMPLEMENTED(); + // Deep-copy the literal boilerplate. + expr->BuildConstantElements(isolate()); + builder() + ->LoadLiteral(expr->constant_elements()) + .CreateArrayLiteral(expr->literal_index(), expr->ComputeFlags(true)); + + TemporaryRegisterScope temporary_register_scope(builder()); + Register index, literal_array; + + // Create nodes to evaluate all the non-constant subexpressions and to store + // them into the newly cloned array. + bool literal_array_in_accumulator = true; + for (int array_index = 0; array_index < expr->values()->length(); + array_index++) { + Expression* subexpr = expr->values()->at(array_index); + if (CompileTimeValue::IsCompileTimeValue(subexpr)) continue; + if (subexpr->IsSpread()) { + // TODO(rmcilroy): Deal with spread expressions. + UNIMPLEMENTED(); + } + + if (literal_array_in_accumulator) { + index = temporary_register_scope.NewRegister(); + literal_array = temporary_register_scope.NewRegister(); + builder()->StoreAccumulatorInRegister(literal_array); + literal_array_in_accumulator = false; + } + + builder() + ->LoadLiteral(Smi::FromInt(array_index)) + .StoreAccumulatorInRegister(index); + Visit(subexpr); + builder()->GenericStoreKeyedProperty(literal_array, index); + } + + if (!literal_array_in_accumulator) { + // Restore literal array into accumulator. + builder()->LoadAccumulatorWithRegister(literal_array); + } } diff --git a/src/interpreter/bytecodes.h b/src/interpreter/bytecodes.h index 1676396562..76f090ef16 100644 --- a/src/interpreter/bytecodes.h +++ b/src/interpreter/bytecodes.h @@ -67,6 +67,8 @@ namespace interpreter { OperandType::kIdx8) \ V(KeyedStoreICStrict, OperandType::kReg8, OperandType::kReg8, \ OperandType::kIdx8) \ + /* TODO(rmcilroy): Remove once literal stores have type feedback slots. */ \ + V(KeyedStoreICGeneric, OperandType::kReg8, OperandType::kReg8) \ \ /* Context operations */ \ V(PushContext, OperandType::kReg8) \ @@ -109,6 +111,9 @@ namespace interpreter { /* Cast operators */ \ V(ToBoolean, OperandType::kNone) \ \ + /* Literals */ \ + V(CreateArrayLiteral, OperandType::kIdx8, OperandType::kImm8) \ + \ /* Closure allocation */ \ V(CreateClosure, OperandType::kImm8) \ \ diff --git a/src/interpreter/interpreter.cc b/src/interpreter/interpreter.cc index 85fa74fc87..98d9b33fb8 100644 --- a/src/interpreter/interpreter.cc +++ b/src/interpreter/interpreter.cc @@ -347,6 +347,26 @@ void Interpreter::DoKeyedStoreICStrict( } +// KeyedStoreICGeneric +// +// Calls the generic KeyStoreIC for and the key with the value in +// the accumulator. +void Interpreter::DoKeyedStoreICGeneric( + compiler::InterpreterAssembler* assembler) { + Callable ic = + CodeFactory::KeyedStoreICInOptimizedCode(isolate_, SLOPPY, MEGAMORPHIC); + Node* code_target = __ HeapConstant(ic.code()); + Node* object_reg_index = __ BytecodeOperandReg8(0); + Node* object = __ LoadRegister(object_reg_index); + Node* name_reg_index = __ BytecodeOperandReg8(1); + Node* name = __ LoadRegister(name_reg_index); + Node* value = __ GetAccumulator(); + Node* result = __ CallIC(ic.descriptor(), code_target, object, name, value); + __ SetAccumulator(result); + __ Dispatch(); +} + + // PushContext // // Pushes the accumulator as the current context, and saves it in @@ -710,6 +730,27 @@ void Interpreter::DoJumpIfFalseConstant( } +// CreateArrayLiteral +// +// Creates an array literal for literal index with flags and +// constant elements in the accumulator. +void Interpreter::DoCreateArrayLiteral( + compiler::InterpreterAssembler* assembler) { + Node* constant_elements = __ GetAccumulator(); + Node* literal_index_raw = __ BytecodeOperandIdx8(0); + Node* literal_index = __ SmiTag(literal_index_raw); + Node* flags_raw = __ BytecodeOperandImm8(1); + Node* flags = __ SmiTag(flags_raw); + Node* closure = __ LoadRegister(Register::function_closure()); + Node* literals_array = + __ LoadObjectField(closure, JSFunction::kLiteralsOffset); + Node* result = __ CallRuntime(Runtime::kCreateArrayLiteral, literals_array, + literal_index, constant_elements, flags); + __ SetAccumulator(result); + __ Dispatch(); +} + + // CreateClosure // // Creates a new closure for SharedFunctionInfo in the accumulator with the diff --git a/test/cctest/interpreter/test-bytecode-generator.cc b/test/cctest/interpreter/test-bytecode-generator.cc index 55fe9e8a54..459fe6232c 100644 --- a/test/cctest/interpreter/test-bytecode-generator.cc +++ b/test/cctest/interpreter/test-bytecode-generator.cc @@ -1944,6 +1944,113 @@ TEST(FunctionLiterals) { } } + +TEST(ArrayLiterals) { + InitializedHandleScope handle_scope; + BytecodeGeneratorHelper helper; + + int simple_flags = + ArrayLiteral::kDisableMementos | ArrayLiteral::kShallowElements; + int deep_elements_flags = ArrayLiteral::kDisableMementos; + ExpectedSnippet snippets[] = { + {"return [ 1, 2 ];", + 0, + 1, + 6, + { + B(LdaConstant), U8(0), // + B(CreateArrayLiteral), U8(0), U8(simple_flags), // + B(Return) // + }, + 1, + {InstanceType::FIXED_ARRAY_TYPE}}, + {"var a = 1; return [ a, a + 1 ];", + 4 * kPointerSize, + 1, + 37, + { + B(LdaSmi8), U8(1), // + B(Star), R(0), // + B(LdaConstant), U8(0), // + B(CreateArrayLiteral), U8(0), U8(3), // + B(Star), R(2), // + B(LdaZero), // + B(Star), R(1), // + B(Ldar), R(0), // + B(KeyedStoreICGeneric), R(2), R(1), // + B(LdaSmi8), U8(1), // + B(Star), R(1), // + B(Ldar), R(0), // + B(Star), R(3), // + B(LdaSmi8), U8(1), // + B(Add), R(3), // + B(KeyedStoreICGeneric), R(2), R(1), // + B(Ldar), R(2), // + B(Return) // + }, + 1, + {InstanceType::FIXED_ARRAY_TYPE}}, + {"return [ [ 1, 2 ], [ 3 ] ];", + 0, + 1, + 6, + { + B(LdaConstant), U8(0), // + B(CreateArrayLiteral), U8(2), U8(deep_elements_flags), // + B(Return) // + }, + 1, + {InstanceType::FIXED_ARRAY_TYPE}}, + {"var a = 1; return [ [ a, 2 ], [ a + 2 ] ];", + 6 * kPointerSize, + 1, + 67, + { + B(LdaSmi8), U8(1), // + B(Star), R(0), // + B(LdaConstant), U8(0), // + B(CreateArrayLiteral), U8(2), U8(deep_elements_flags), // + B(Star), R(2), // + B(LdaZero), // + B(Star), R(1), // + B(LdaConstant), U8(1), // + B(CreateArrayLiteral), U8(0), U8(simple_flags), // + B(Star), R(4), // + B(LdaZero), // + B(Star), R(3), // + B(Ldar), R(0), // + B(KeyedStoreICGeneric), R(4), R(3), // + B(Ldar), R(4), // + B(KeyedStoreICGeneric), R(2), R(1), // + B(LdaSmi8), U8(1), // + B(Star), R(1), // + B(LdaConstant), U8(2), // + B(CreateArrayLiteral), U8(1), U8(simple_flags), // + B(Star), R(4), // + B(LdaZero), // + B(Star), R(3), // + B(Ldar), R(0), // + B(Star), R(5), // + B(LdaSmi8), U8(2), // + B(Add), R(5), // + B(KeyedStoreICGeneric), R(4), R(3), // + B(Ldar), R(4), // + B(KeyedStoreICGeneric), R(2), R(1), // + B(Ldar), R(2), // + B(Return), // + }, + 3, + {InstanceType::FIXED_ARRAY_TYPE, InstanceType::FIXED_ARRAY_TYPE, + InstanceType::FIXED_ARRAY_TYPE}}, + }; + + for (size_t i = 0; i < arraysize(snippets); i++) { + Handle bytecode_array = + helper.MakeBytecodeForFunctionBody(snippets[i].code_snippet); + CheckBytecodeArrayEqual(snippets[i], bytecode_array); + } +} + } // namespace interpreter } // namespace internal } // namespace v8 diff --git a/test/cctest/interpreter/test-interpreter.cc b/test/cctest/interpreter/test-interpreter.cc index 188b3e61d9..7d2f527fe2 100644 --- a/test/cctest/interpreter/test-interpreter.cc +++ b/test/cctest/interpreter/test-interpreter.cc @@ -102,6 +102,10 @@ class InterpreterTester { return isolate->factory()->string_table()->LookupString(isolate, result); } + static std::string SourceForBody(const char* body) { + return "function " + function_name() + "() {\n" + std::string(body) + "\n}"; + } + static std::string function_name() { return std::string(kFunctionName); } @@ -1637,3 +1641,34 @@ TEST(InterpreterFunctionLiteral) { Handle(Smi::FromInt(3), handles.main_isolate())).ToHandleChecked(); CHECK_EQ(Smi::cast(*return_val), Smi::FromInt(5)); } + + +TEST(InterpreterArrayLiterals) { + HandleAndZoneScope handles; + i::Isolate* isolate = handles.main_isolate(); + i::Factory* factory = isolate->factory(); + + std::pair> literals[6] = { + std::make_pair("return [][0];\n", + factory->undefined_value()), + std::make_pair("return [1, 3, 2][1];\n", + Handle(Smi::FromInt(3), isolate)), + std::make_pair("return ['a', 'b', 'c'][2];\n", + factory->NewStringFromStaticChars("c")), + std::make_pair("var a = 100; return [a, a + 1, a + 2, a + 3][2];\n", + Handle(Smi::FromInt(102), isolate)), + std::make_pair("return [[1, 2, 3], ['a', 'b', 'c']][1][0];\n", + factory->NewStringFromStaticChars("a")), + std::make_pair("var t = 't'; return [[t, t + 'est'], [1 + t]][0][1];\n", + factory->NewStringFromStaticChars("test")) + }; + + for (size_t i = 0; i < arraysize(literals); i++) { + std::string source(InterpreterTester::SourceForBody(literals[i].first)); + InterpreterTester tester(handles.main_isolate(), source.c_str()); + auto callable = tester.GetCallable<>(); + + Handle return_value = callable().ToHandleChecked(); + CHECK(return_value->SameValue(*literals[i].second)); + } +} diff --git a/test/unittests/interpreter/bytecode-array-builder-unittest.cc b/test/unittests/interpreter/bytecode-array-builder-unittest.cc index 32acf9f15c..032c8c1da9 100644 --- a/test/unittests/interpreter/bytecode-array-builder-unittest.cc +++ b/test/unittests/interpreter/bytecode-array-builder-unittest.cc @@ -57,11 +57,15 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) { .LoadNamedProperty(reg, 0, LanguageMode::STRICT) .LoadKeyedProperty(reg, 0, LanguageMode::STRICT) .StoreNamedProperty(reg, reg, 0, LanguageMode::STRICT) - .StoreKeyedProperty(reg, reg, 0, LanguageMode::STRICT); + .StoreKeyedProperty(reg, reg, 0, LanguageMode::STRICT) + .GenericStoreKeyedProperty(reg, reg); // Emit closure operations. builder.CreateClosure(NOT_TENURED); + // Emit literal creation operations + builder.CreateArrayLiteral(0, 0); + // Call operations. builder.Call(reg, reg, 0); builder.CallRuntime(Runtime::kIsArray, reg, 1);