From: bmeurer Date: Mon, 1 Jun 2015 07:20:50 +0000 (-0700) Subject: [turbofan] First step towards sanitizing for-in and making it optimizable. X-Git-Tag: upstream/4.7.83~2329 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e2e47f30be06e6ea6b8a3cc90138381914ecd07d;p=platform%2Fupstream%2Fv8.git [turbofan] First step towards sanitizing for-in and making it optimizable. In a nutshell: The FILTER_KEY builtin is gone, and was replaced by a simple runtime call to ForInFilter, which does everything and is even cheaper (because FILTER_KEY used to call into the runtime anyway). And ForInFilter returns either the name or undefined, which makes it possible to remove the control flow construction from the AstGraphBuilder, and thereby make both the initialization and the per-loop code of for-in optimizable later (in typed lowering). R=jarin@chromium.org Review URL: https://codereview.chromium.org/1160983004 Cr-Commit-Position: refs/heads/master@{#28711} --- diff --git a/BUILD.gn b/BUILD.gn index 9740ff3..8d56eac 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1003,6 +1003,7 @@ source_set("v8_base") { "src/runtime/runtime-compiler.cc", "src/runtime/runtime-date.cc", "src/runtime/runtime-debug.cc", + "src/runtime/runtime-forin.cc", "src/runtime/runtime-function.cc", "src/runtime/runtime-generator.cc", "src/runtime/runtime-i18n.cc", diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 1446aeb..7a0d9b6 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -1271,9 +1271,11 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { // just skip it. __ push(r1); // Enumerable. __ push(r3); // Current entry. - __ InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION); + __ CallRuntime(Runtime::kForInFilter, 2); PrepareForBailoutForId(stmt->FilterId(), TOS_REG); - __ mov(r3, Operand(r0), SetCC); + __ mov(r3, Operand(r0)); + __ LoadRoot(ip, Heap::kUndefinedValueRootIndex); + __ cmp(r0, ip); __ b(eq, loop_statement.continue_label()); // Update the 'each' property or variable from the possibly filtered diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index 9441cb7..0e6391f 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -1254,10 +1254,11 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { // any more. If the property has been removed while iterating, we // just skip it. __ Push(x1, x3); - __ InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION); + __ CallRuntime(Runtime::kForInFilter, 2); PrepareForBailoutForId(stmt->FilterId(), TOS_REG); __ Mov(x3, x0); - __ Cbz(x0, loop_statement.continue_label()); + __ JumpIfRoot(x0, Heap::kUndefinedValueRootIndex, + loop_statement.continue_label()); // Update the 'each' property or variable from the possibly filtered // entry in register x3. diff --git a/src/builtins.h b/src/builtins.h index 59cd894..7d0d6c2 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -194,7 +194,6 @@ enum BuiltinExtraArguments { V(DELETE, 2) \ V(IN, 1) \ V(INSTANCE_OF, 1) \ - V(FILTER_KEY, 1) \ V(CALL_NON_FUNCTION, 0) \ V(CALL_NON_FUNCTION_AS_CONSTRUCTOR, 0) \ V(CALL_FUNCTION_PROXY, 1) \ diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index fb848c9..78a0906 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -1292,148 +1292,80 @@ void AstGraphBuilder::VisitForStatement(ForStatement* stmt) { } -// TODO(dcarney): this is a big function. Try to clean up some. void AstGraphBuilder::VisitForInStatement(ForInStatement* stmt) { VisitForValue(stmt->subject()); - Node* obj = environment()->Pop(); - // Check for undefined or null before entering loop. - IfBuilder is_undefined(this); - Node* is_undefined_cond = - NewNode(javascript()->StrictEqual(), obj, jsgraph()->UndefinedConstant()); - is_undefined.If(is_undefined_cond); - is_undefined.Then(); - is_undefined.Else(); + Node* object = environment()->Pop(); + BlockBuilder for_block(this); + for_block.BeginBlock(); + // Check for null or undefined before entering loop. + Node* is_null_cond = + NewNode(javascript()->StrictEqual(), object, jsgraph()->NullConstant()); + for_block.BreakWhen(is_null_cond, BranchHint::kFalse); + Node* is_undefined_cond = NewNode(javascript()->StrictEqual(), object, + jsgraph()->UndefinedConstant()); + for_block.BreakWhen(is_undefined_cond, BranchHint::kFalse); { - IfBuilder is_null(this); - Node* is_null_cond = - NewNode(javascript()->StrictEqual(), obj, jsgraph()->NullConstant()); - is_null.If(is_null_cond); - is_null.Then(); - is_null.Else(); // Convert object to jsobject. - // PrepareForBailoutForId(stmt->PrepareId(), TOS_REG); - obj = NewNode(javascript()->ToObject(), obj); - PrepareFrameState(obj, stmt->ToObjectId(), OutputFrameStateCombine::Push()); - environment()->Push(obj); - // TODO(dcarney): should do a fast enum cache check here to skip runtime. - Node* cache_type = NewNode( - javascript()->CallRuntime(Runtime::kGetPropertyNamesFast, 1), obj); - PrepareFrameState(cache_type, stmt->EnumId(), - OutputFrameStateCombine::Push()); - // TODO(dcarney): these next runtime calls should be removed in favour of - // a few simplified instructions. - Node* cache_pair = NewNode( - javascript()->CallRuntime(Runtime::kForInInit, 2), obj, cache_type); - // cache_type may have been replaced. - Node* cache_array = NewNode(common()->Projection(0), cache_pair); - cache_type = NewNode(common()->Projection(1), cache_pair); - Node* cache_length = - NewNode(javascript()->CallRuntime(Runtime::kForInCacheArrayLength, 2), - cache_type, cache_array); + object = BuildToObject(object, stmt->ToObjectId()); + environment()->Push(object); + + // Prepare for-in cache. + Node* prepare = NewNode(javascript()->ForInPrepare(), object); + PrepareFrameState(prepare, stmt->EnumId(), OutputFrameStateCombine::Push()); + Node* cache_type = NewNode(common()->Projection(0), prepare); + Node* cache_array = NewNode(common()->Projection(1), prepare); + Node* cache_length = NewNode(common()->Projection(2), prepare); + + // Construct the rest of the environment. + environment()->Push(cache_type); + environment()->Push(cache_array); + environment()->Push(cache_length); + environment()->Push(jsgraph()->ZeroConstant()); + + // Build the actual loop body. + LoopBuilder for_loop(this); + for_loop.BeginLoop(GetVariablesAssignedInLoop(stmt), CheckOsrEntry(stmt)); { - // Construct the rest of the environment. - environment()->Push(cache_type); - environment()->Push(cache_array); - environment()->Push(cache_length); - environment()->Push(jsgraph()->ZeroConstant()); - - // Build the actual loop body. - VisitForInBody(stmt); - } - is_null.End(); - } - is_undefined.End(); -} - - -// TODO(dcarney): this is a big function. Try to clean up some. -void AstGraphBuilder::VisitForInBody(ForInStatement* stmt) { - LoopBuilder for_loop(this); - for_loop.BeginLoop(GetVariablesAssignedInLoop(stmt), CheckOsrEntry(stmt)); - - // These stack values are renamed in the case of OSR, so reload them - // from the environment. - Node* index = environment()->Peek(0); - Node* cache_length = environment()->Peek(1); - Node* cache_array = environment()->Peek(2); - Node* cache_type = environment()->Peek(3); - Node* obj = environment()->Peek(4); - - // Check loop termination condition (cannot deoptimize). - { - FrameStateBeforeAndAfter states(this, BailoutId::None()); - Node* exit_cond = NewNode(javascript()->LessThan(LanguageMode::SLOPPY), - index, cache_length); - states.AddToNode(exit_cond, BailoutId::None(), - OutputFrameStateCombine::Ignore()); - for_loop.BreakUnless(exit_cond); - } - Node* pair = NewNode(javascript()->CallRuntime(Runtime::kForInNext, 4), obj, - cache_array, cache_type, index); - Node* value = NewNode(common()->Projection(0), pair); - Node* should_filter = NewNode(common()->Projection(1), pair); - environment()->Push(value); - { - // Test if FILTER_KEY needs to be called. - IfBuilder test_should_filter(this); - Node* should_filter_cond = NewNode( - javascript()->StrictEqual(), should_filter, jsgraph()->TrueConstant()); - test_should_filter.If(should_filter_cond); - test_should_filter.Then(); - value = environment()->Pop(); - Node* builtins = BuildLoadBuiltinsObject(); - Node* function = BuildLoadObjectField( - builtins, - JSBuiltinsObject::OffsetOfFunctionWithId(Builtins::FILTER_KEY)); - // result is either the string key or Smi(0) indicating the property - // is gone. - Node* res = NewNode( - javascript()->CallFunction(3, NO_CALL_FUNCTION_FLAGS, language_mode()), - function, obj, value); - PrepareFrameState(res, stmt->FilterId(), OutputFrameStateCombine::Push()); - Node* property_missing = - NewNode(javascript()->StrictEqual(), res, jsgraph()->ZeroConstant()); - { - IfBuilder is_property_missing(this); - is_property_missing.If(property_missing); - is_property_missing.Then(); - // Inc counter and continue (cannot deoptimize). + // These stack values are renamed in the case of OSR, so reload them + // from the environment. + Node* index = environment()->Peek(0); + Node* cache_length = environment()->Peek(1); + Node* cache_array = environment()->Peek(2); + Node* cache_type = environment()->Peek(3); + Node* object = environment()->Peek(4); + + // Check loop termination condition. + Node* exit_cond = NewNode(javascript()->ForInDone(), index, cache_length); + for_loop.BreakWhen(exit_cond); + + // Compute the next enumerated value. + Node* value = NewNode(javascript()->ForInNext(), object, cache_array, + cache_type, index); + PrepareFrameState(value, stmt->FilterId(), + OutputFrameStateCombine::Push()); + IfBuilder test_value(this); + Node* test_value_cond = NewNode(javascript()->StrictEqual(), value, + jsgraph()->UndefinedConstant()); + test_value.If(test_value_cond, BranchHint::kFalse); + test_value.Then(); + test_value.Else(); { - FrameStateBeforeAndAfter states(this, BailoutId::None()); - Node* index_inc = NewNode(javascript()->Add(LanguageMode::SLOPPY), - index, jsgraph()->OneConstant()); - states.AddToNode(index_inc, BailoutId::None(), - OutputFrameStateCombine::Ignore()); - environment()->Poke(0, index_inc); + // Bind value and do loop body. + VisitForInAssignment(stmt->each(), value, stmt->AssignmentId()); + VisitIterationBody(stmt, &for_loop); } - for_loop.Continue(); - is_property_missing.Else(); - is_property_missing.End(); - } - // Replace 'value' in environment. - environment()->Push(res); - test_should_filter.Else(); - test_should_filter.End(); - } - value = environment()->Pop(); - // Bind value and do loop body. - VisitForInAssignment(stmt->each(), value, stmt->AssignmentId()); - VisitIterationBody(stmt, &for_loop); - index = environment()->Peek(0); - for_loop.EndBody(); + test_value.End(); + index = environment()->Peek(0); + for_loop.EndBody(); - // Inc counter and continue (cannot deoptimize). - { - FrameStateBeforeAndAfter states(this, BailoutId::None()); - Node* index_inc = NewNode(javascript()->Add(LanguageMode::SLOPPY), index, - jsgraph()->OneConstant()); - states.AddToNode(index_inc, BailoutId::None(), - OutputFrameStateCombine::Ignore()); - environment()->Poke(0, index_inc); + // Increment counter and continue. + index = NewNode(javascript()->ForInStep(), index); + environment()->Poke(0, index); + } + for_loop.EndLoop(); + environment()->Drop(5); } - for_loop.EndLoop(); - environment()->Drop(5); - // PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS); + for_block.EndBlock(); } @@ -3339,6 +3271,13 @@ Node* AstGraphBuilder::BuildToName(Node* input, BailoutId bailout_id) { } +Node* AstGraphBuilder::BuildToObject(Node* input, BailoutId bailout_id) { + Node* object = NewNode(javascript()->ToObject(), input); + PrepareFrameState(object, bailout_id, OutputFrameStateCombine::Push()); + return object; +} + + Node* AstGraphBuilder::BuildSetHomeObject(Node* value, Node* home_object, Expression* expr) { if (!FunctionLiteral::NeedsHomeObject(expr)) return value; diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index eaaf2a8..b9ce500 100644 --- a/src/compiler/ast-graph-builder.h +++ b/src/compiler/ast-graph-builder.h @@ -288,8 +288,9 @@ class AstGraphBuilder : public AstVisitor { Node* BuildStoreExternal(ExternalReference ref, MachineType type, Node* val); // Builders for automatic type conversion. - Node* BuildToBoolean(Node* value); - Node* BuildToName(Node* value, BailoutId bailout_id); + Node* BuildToBoolean(Node* input); + Node* BuildToName(Node* input, BailoutId bailout_id); + Node* BuildToObject(Node* input, BailoutId bailout_id); // Builder for adding the [[HomeObject]] to a value if the value came from a // function literal and needs a home object. Do nothing otherwise. @@ -359,7 +360,6 @@ class AstGraphBuilder : public AstVisitor { // Dispatched from VisitForInStatement. void VisitForInAssignment(Expression* expr, Node* value, BailoutId bailout_id); - void VisitForInBody(ForInStatement* stmt); // Dispatched from VisitClassLiteral. void VisitClassLiteralContents(ClassLiteral* expr); diff --git a/src/compiler/control-builders.cc b/src/compiler/control-builders.cc index 0e4f168..6bc3649 100644 --- a/src/compiler/control-builders.cc +++ b/src/compiler/control-builders.cc @@ -143,6 +143,16 @@ void BlockBuilder::Break() { } +void BlockBuilder::BreakWhen(Node* condition, BranchHint hint) { + IfBuilder control_if(builder_); + control_if.If(condition, hint); + control_if.Then(); + Break(); + control_if.Else(); + control_if.End(); +} + + void BlockBuilder::EndBlock() { break_environment_->Merge(environment()); set_environment(break_environment_); diff --git a/src/compiler/control-builders.h b/src/compiler/control-builders.h index c85714e..9f3afce 100644 --- a/src/compiler/control-builders.h +++ b/src/compiler/control-builders.h @@ -131,6 +131,9 @@ class BlockBuilder final : public ControlBuilder { // Primitive support for break. void Break() final; + // Compound control commands for conditional break. + void BreakWhen(Node* condition, BranchHint = BranchHint::kNone); + private: Environment* break_environment_; // Environment after the block exits. }; diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index 5fe37e5..feb3d03 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/src/compiler/js-generic-lowering.cc @@ -558,6 +558,208 @@ void JSGenericLowering::LowerJSCallRuntime(Node* node) { } +void JSGenericLowering::LowerJSForInDone(Node* node) { + ReplaceWithRuntimeCall(node, Runtime::kForInDone); +} + + +void JSGenericLowering::LowerJSForInNext(Node* node) { + ReplaceWithRuntimeCall(node, Runtime::kForInNext); +} + + +void JSGenericLowering::LowerJSForInPrepare(Node* node) { + Node* object = NodeProperties::GetValueInput(node, 0); + Node* context = NodeProperties::GetContextInput(node); + Node* effect = NodeProperties::GetEffectInput(node); + Node* control = NodeProperties::GetControlInput(node); + Node* frame_state = NodeProperties::GetFrameStateInput(node, 0); + + // Get the set of properties to enumerate. + Runtime::Function const* function = + Runtime::FunctionForId(Runtime::kGetPropertyNamesFast); + CallDescriptor const* descriptor = Linkage::GetRuntimeCallDescriptor( + zone(), function->function_id, 1, Operator::kNoProperties); + Node* cache_type = effect = graph()->NewNode( + common()->Call(descriptor), + jsgraph()->CEntryStubConstant(function->result_size), object, + jsgraph()->ExternalConstant( + ExternalReference(function->function_id, isolate())), + jsgraph()->Int32Constant(1), context, frame_state, effect, control); + control = graph()->NewNode(common()->IfSuccess(), cache_type); + + Node* object_map = effect = graph()->NewNode( + machine()->Load(kMachAnyTagged), object, + jsgraph()->IntPtrConstant(HeapObject::kMapOffset - kHeapObjectTag), + effect, control); + Node* cache_type_map = effect = graph()->NewNode( + machine()->Load(kMachAnyTagged), cache_type, + jsgraph()->IntPtrConstant(HeapObject::kMapOffset - kHeapObjectTag), + effect, control); + Node* meta_map = jsgraph()->HeapConstant(isolate()->factory()->meta_map()); + + // If we got a map from the GetPropertyNamesFast runtime call, we can do a + // fast modification check. Otherwise, we got a fixed array, and we have to + // perform a slow check on every iteration. + Node* check0 = + graph()->NewNode(machine()->WordEqual(), cache_type_map, meta_map); + Node* branch0 = + graph()->NewNode(common()->Branch(BranchHint::kTrue), check0, control); + + Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0); + Node* cache_array_true0; + Node* cache_length_true0; + Node* cache_type_true0; + Node* etrue0; + { + // Enum cache case. + Node* cache_type_enum_length = etrue0 = graph()->NewNode( + machine()->Load(kMachUint32), cache_type, + jsgraph()->IntPtrConstant(Map::kBitField3Offset - kHeapObjectTag), + effect, if_true0); + cache_type_enum_length = + graph()->NewNode(machine()->Word32And(), cache_type_enum_length, + jsgraph()->Uint32Constant(Map::EnumLengthBits::kMask)); + + Node* check1 = + graph()->NewNode(machine()->Word32Equal(), cache_type_enum_length, + jsgraph()->Int32Constant(0)); + Node* branch1 = + graph()->NewNode(common()->Branch(BranchHint::kTrue), check1, if_true0); + + Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1); + Node* cache_array_true1; + Node* etrue1; + { + // No properties to enumerate. + cache_array_true1 = + jsgraph()->HeapConstant(isolate()->factory()->empty_fixed_array()); + etrue1 = etrue0; + } + + Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1); + Node* cache_array_false1; + Node* efalse1; + { + // Load the enumeration cache from the instance descriptors of {object}. + Node* object_map_descriptors = efalse1 = graph()->NewNode( + machine()->Load(kMachAnyTagged), object_map, + jsgraph()->IntPtrConstant(Map::kDescriptorsOffset - kHeapObjectTag), + etrue0, if_false1); + Node* object_map_enum_cache = efalse1 = graph()->NewNode( + machine()->Load(kMachAnyTagged), object_map_descriptors, + jsgraph()->IntPtrConstant(DescriptorArray::kEnumCacheOffset - + kHeapObjectTag), + efalse1, if_false1); + cache_array_false1 = efalse1 = graph()->NewNode( + machine()->Load(kMachAnyTagged), object_map_enum_cache, + jsgraph()->IntPtrConstant( + DescriptorArray::kEnumCacheBridgeCacheOffset - kHeapObjectTag), + efalse1, if_false1); + } + + if_true0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1); + etrue0 = + graph()->NewNode(common()->EffectPhi(2), etrue1, efalse1, if_true0); + cache_array_true0 = + graph()->NewNode(common()->Phi(kMachAnyTagged, 2), cache_array_true1, + cache_array_false1, if_true0); + + cache_length_true0 = graph()->NewNode( + machine()->WordShl(), + machine()->Is64() + ? graph()->NewNode(machine()->ChangeUint32ToUint64(), + cache_type_enum_length) + : cache_type_enum_length, + jsgraph()->Int32Constant(kSmiShiftSize + kSmiTagSize)); + cache_type_true0 = cache_type; + } + + Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0); + Node* cache_array_false0; + Node* cache_length_false0; + Node* cache_type_false0; + Node* efalse0; + { + // FixedArray case. + Node* object_instance_type = efalse0 = graph()->NewNode( + machine()->Load(kMachUint8), object_map, + jsgraph()->IntPtrConstant(Map::kInstanceTypeOffset - kHeapObjectTag), + effect, if_false0); + + STATIC_ASSERT(FIRST_JS_PROXY_TYPE == FIRST_SPEC_OBJECT_TYPE); + Node* check1 = graph()->NewNode( + machine()->Uint32LessThanOrEqual(), object_instance_type, + jsgraph()->Uint32Constant(LAST_JS_PROXY_TYPE)); + Node* branch1 = graph()->NewNode(common()->Branch(BranchHint::kFalse), + check1, if_false0); + + Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1); + Node* cache_type_true1 = jsgraph()->ZeroConstant(); // Zero indicates proxy + + Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1); + Node* cache_type_false1 = jsgraph()->OneConstant(); // One means slow check + + if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1); + cache_type_false0 = + graph()->NewNode(common()->Phi(kMachAnyTagged, 2), cache_type_true1, + cache_type_false1, if_false0); + + cache_array_false0 = cache_type; + cache_length_false0 = efalse0 = graph()->NewNode( + machine()->Load(kMachAnyTagged), cache_array_false0, + jsgraph()->IntPtrConstant(FixedArray::kLengthOffset - kHeapObjectTag), + efalse0, if_false0); + } + + control = graph()->NewNode(common()->Merge(2), if_true0, if_false0); + effect = graph()->NewNode(common()->EffectPhi(2), etrue0, efalse0, control); + Node* cache_array = + graph()->NewNode(common()->Phi(kMachAnyTagged, 2), cache_array_true0, + cache_array_false0, control); + Node* cache_length = + graph()->NewNode(common()->Phi(kMachAnyTagged, 2), cache_length_true0, + cache_length_false0, control); + cache_type = graph()->NewNode(common()->Phi(kMachAnyTagged, 2), + cache_type_true0, cache_type_false0, control); + + for (auto edge : node->use_edges()) { + if (NodeProperties::IsEffectEdge(edge)) { + edge.UpdateTo(effect); + } else if (NodeProperties::IsControlEdge(edge)) { + Node* const use = edge.from(); + DCHECK_EQ(IrOpcode::kIfSuccess, use->opcode()); + use->ReplaceUses(control); + use->Kill(); + } else { + Node* const use = edge.from(); + DCHECK(NodeProperties::IsValueEdge(edge)); + DCHECK_EQ(IrOpcode::kProjection, use->opcode()); + switch (ProjectionIndexOf(use->op())) { + case 0: + use->ReplaceUses(cache_type); + break; + case 1: + use->ReplaceUses(cache_array); + break; + case 2: + use->ReplaceUses(cache_length); + break; + default: + UNREACHABLE(); + break; + } + use->Kill(); + } + } +} + + +void JSGenericLowering::LowerJSForInStep(Node* node) { + ReplaceWithRuntimeCall(node, Runtime::kForInStep); +} + + void JSGenericLowering::LowerJSStackCheck(Node* node) { Node* effect = NodeProperties::GetEffectInput(node); Node* control = NodeProperties::GetControlInput(node); diff --git a/src/compiler/js-operator.cc b/src/compiler/js-operator.cc index 37fd547..87df6a0 100644 --- a/src/compiler/js-operator.cc +++ b/src/compiler/js-operator.cc @@ -238,6 +238,10 @@ const CreateClosureParameters& CreateClosureParametersOf(const Operator* op) { V(HasProperty, Operator::kNoProperties, 2, 1) \ V(TypeOf, Operator::kPure, 1, 1) \ V(InstanceOf, Operator::kNoProperties, 2, 1) \ + V(ForInDone, Operator::kPure, 2, 1) \ + V(ForInNext, Operator::kNoProperties, 4, 1) \ + V(ForInPrepare, Operator::kNoProperties, 1, 3) \ + V(ForInStep, Operator::kPure, 1, 1) \ V(StackCheck, Operator::kNoProperties, 0, 0) \ V(CreateFunctionContext, Operator::kNoProperties, 1, 1) \ V(CreateWithContext, Operator::kNoProperties, 2, 1) \ diff --git a/src/compiler/js-operator.h b/src/compiler/js-operator.h index 3678658..ae26665 100644 --- a/src/compiler/js-operator.h +++ b/src/compiler/js-operator.h @@ -300,6 +300,11 @@ class JSOperatorBuilder final : public ZoneObject { const Operator* TypeOf(); const Operator* InstanceOf(); + const Operator* ForInDone(); + const Operator* ForInNext(); + const Operator* ForInPrepare(); + const Operator* ForInStep(); + const Operator* StackCheck(); // TODO(titzer): nail down the static parts of each of these context flavors. diff --git a/src/compiler/linkage.cc b/src/compiler/linkage.cc index 881134f..24f5e49 100644 --- a/src/compiler/linkage.cc +++ b/src/compiler/linkage.cc @@ -114,9 +114,8 @@ bool Linkage::NeedsFrameState(Runtime::FunctionId function) { case Runtime::kDefineClassMethod: // TODO(jarin): Is it safe? case Runtime::kDefineGetterPropertyUnchecked: // TODO(jarin): Is it safe? case Runtime::kDefineSetterPropertyUnchecked: // TODO(jarin): Is it safe? - case Runtime::kForInCacheArrayLength: - case Runtime::kForInInit: - case Runtime::kForInNext: + case Runtime::kForInDone: + case Runtime::kForInStep: case Runtime::kNewArguments: case Runtime::kNewClosure: case Runtime::kNewFunctionContext: diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index 5ccbe4e..bcc1994 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -134,6 +134,10 @@ V(JSCallConstruct) \ V(JSCallFunction) \ V(JSCallRuntime) \ + V(JSForInDone) \ + V(JSForInNext) \ + V(JSForInPrepare) \ + V(JSForInStep) \ V(JSYield) \ V(JSStackCheck) diff --git a/src/compiler/operator-properties.cc b/src/compiler/operator-properties.cc index 7ea0405..ed926a7 100644 --- a/src/compiler/operator-properties.cc +++ b/src/compiler/operator-properties.cc @@ -58,6 +58,8 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { case IrOpcode::kJSToName: // Misc operations + case IrOpcode::kJSForInNext: + case IrOpcode::kJSForInPrepare: case IrOpcode::kJSStackCheck: case IrOpcode::kJSDeleteProperty: return 1; diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index dafd3c9..782c8e3 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -1602,6 +1602,30 @@ Bounds Typer::Visitor::TypeJSCallRuntime(Node* node) { } +Bounds Typer::Visitor::TypeJSForInNext(Node* node) { + return Bounds(Type::None(zone()), + Type::Union(Type::Name(), Type::Undefined(), zone())); +} + + +Bounds Typer::Visitor::TypeJSForInPrepare(Node* node) { + // TODO(bmeurer): Return a tuple type here. + return Bounds::Unbounded(zone()); +} + + +Bounds Typer::Visitor::TypeJSForInDone(Node* node) { + return Bounds(Type::None(zone()), Type::Boolean(zone())); +} + + +Bounds Typer::Visitor::TypeJSForInStep(Node* node) { + STATIC_ASSERT(Map::EnumLengthBits::kMax <= FixedArray::kMaxLength); + return Bounds(Type::None(zone()), + Type::Range(1, FixedArray::kMaxLength + 1, zone())); +} + + Bounds Typer::Visitor::TypeJSStackCheck(Node* node) { return Bounds::Unbounded(zone()); } diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc index d827f15..6ea52b0 100644 --- a/src/compiler/verifier.cc +++ b/src/compiler/verifier.cc @@ -564,6 +564,25 @@ void Verifier::Visitor::Check(Node* node) { CheckUpperIs(node, Type::Any()); break; + case IrOpcode::kJSForInPrepare: { + // TODO(bmeurer): What are the constraints on thse? + CheckUpperIs(node, Type::Any()); + break; + } + case IrOpcode::kJSForInDone: { + CheckValueInputIs(node, 0, Type::UnsignedSmall()); + break; + } + case IrOpcode::kJSForInNext: { + CheckUpperIs(node, Type::Union(Type::Name(), Type::Undefined())); + break; + } + case IrOpcode::kJSForInStep: { + CheckValueInputIs(node, 0, Type::UnsignedSmall()); + CheckUpperIs(node, Type::UnsignedSmall()); + break; + } + case IrOpcode::kJSStackCheck: // Type is empty. CheckNotTyped(node); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 8af0c33..30406d6 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5201,12 +5201,17 @@ void HOptimizedGraphBuilder::BuildForInBody(ForInStatement* stmt, Add(enumerable, environment()->ExpressionStackAt(3)); Bind(each_var, key); } else { - HValue* function = AddLoadJSBuiltin(Builtins::FILTER_KEY); Add(enumerable, key); - key = Add(function, 2); + Runtime::FunctionId function_id = Runtime::kForInFilter; + key = Add(isolate()->factory()->empty_string(), + Runtime::FunctionForId(function_id), 2); Bind(each_var, key); Add(stmt->AssignmentId()); - Add(key); + IfBuilder if_undefined(this); + if_undefined.If(key, + graph()->GetConstantUndefined()); + if_undefined.ThenDeopt(Deoptimizer::kUndefined); + if_undefined.End(); } BreakAndContinueInfo break_info(stmt, scope(), 5); diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 700ccfe..573ca70 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -1197,9 +1197,9 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { // just skip it. __ push(ecx); // Enumerable. __ push(ebx); // Current entry. - __ InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION); + __ CallRuntime(Runtime::kForInFilter, 2); PrepareForBailoutForId(stmt->FilterId(), TOS_REG); - __ test(eax, eax); + __ cmp(eax, isolate()->factory()->undefined_value()); __ j(equal, loop_statement.continue_label()); __ mov(ebx, eax); diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 0c00f71..c81f346 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -1259,10 +1259,11 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { // any more. If the property has been removed while iterating, we // just skip it. __ Push(a1, a3); // Enumerable and current entry. - __ InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION); + __ CallRuntime(Runtime::kForInFilter, 2); PrepareForBailoutForId(stmt->FilterId(), TOS_REG); __ mov(a3, result_register()); - __ Branch(loop_statement.continue_label(), eq, a3, Operand(zero_reg)); + __ LoadRoot(at, Heap::kUndefinedValueRootIndex); + __ Branch(loop_statement.continue_label(), eq, a3, Operand(at)); // Update the 'each' property or variable from the possibly filtered // entry in register a3. diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc index e3983f7..29caab4 100644 --- a/src/mips64/full-codegen-mips64.cc +++ b/src/mips64/full-codegen-mips64.cc @@ -1256,10 +1256,11 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { // any more. If the property has been removed while iterating, we // just skip it. __ Push(a1, a3); // Enumerable and current entry. - __ InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION); + __ CallRuntime(Runtime::kForInFilter, 2); PrepareForBailoutForId(stmt->FilterId(), TOS_REG); __ mov(a3, result_register()); - __ Branch(loop_statement.continue_label(), eq, a3, Operand(zero_reg)); + __ LoadRoot(at, Heap::kUndefinedValueRootIndex); + __ Branch(loop_statement.continue_label(), eq, a3, Operand(at)); // Update the 'each' property or variable from the possibly filtered // entry in register a3. diff --git a/src/runtime.js b/src/runtime.js index d1ebc30..b600778 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -49,7 +49,6 @@ var SHR_STRONG; var DELETE; var IN; var INSTANCE_OF; -var FILTER_KEY; var CALL_NON_FUNCTION; var CALL_NON_FUNCTION_AS_CONSTRUCTOR; var CALL_FUNCTION_PROXY; @@ -561,16 +560,6 @@ INSTANCE_OF = function INSTANCE_OF(F) { } -// Filter a given key against an object by checking if the object -// has a property with the given key; return the key as a string if -// it has. Otherwise returns 0 (smi). Used in for-in statements. -FILTER_KEY = function FILTER_KEY(key) { - var string = %$toName(key); - if (%HasProperty(this, string)) return string; - return 0; -} - - CALL_NON_FUNCTION = function CALL_NON_FUNCTION() { var delegate = %GetFunctionDelegate(this); if (!IS_FUNCTION(delegate)) { diff --git a/src/runtime/runtime-array.cc b/src/runtime/runtime-array.cc index 06cfa05..cc4a2e7 100644 --- a/src/runtime/runtime-array.cc +++ b/src/runtime/runtime-array.cc @@ -1302,87 +1302,6 @@ RUNTIME_FUNCTION(Runtime_HasComplexElements) { } -// TODO(dcarney): remove this function when TurboFan supports it. -// Takes the object to be iterated over and the result of GetPropertyNamesFast -// Returns pair (cache_array, cache_type). -RUNTIME_FUNCTION_RETURN_PAIR(Runtime_ForInInit) { - SealHandleScope scope(isolate); - DCHECK(args.length() == 2); - // This simulates CONVERT_ARG_HANDLE_CHECKED for calls returning pairs. - // Not worth creating a macro atm as this function should be removed. - if (!args[0]->IsJSReceiver() || !args[1]->IsObject()) { - Object* error = isolate->ThrowIllegalOperation(); - return MakePair(error, isolate->heap()->undefined_value()); - } - Handle object = args.at(0); - Handle cache_type = args.at(1); - if (cache_type->IsMap()) { - // Enum cache case. - return MakePair(Map::cast(*cache_type)->EnumLength() != 0 - ? object->map()->instance_descriptors()->GetEnumCache() - : isolate->heap()->empty_fixed_array(), - *cache_type); - } else { - // FixedArray case. - Smi* new_cache_type = Smi::FromInt(object->IsJSProxy() ? 0 : 1); - return MakePair(*Handle::cast(cache_type), new_cache_type); - } -} - - -// TODO(dcarney): remove this function when TurboFan supports it. -RUNTIME_FUNCTION(Runtime_ForInCacheArrayLength) { - SealHandleScope shs(isolate); - DCHECK(args.length() == 2); - CONVERT_ARG_HANDLE_CHECKED(Object, cache_type, 0); - CONVERT_ARG_HANDLE_CHECKED(FixedArray, array, 1); - int length = 0; - if (cache_type->IsMap()) { - length = Map::cast(*cache_type)->EnumLength(); - } else { - DCHECK(cache_type->IsSmi()); - length = array->length(); - } - return Smi::FromInt(length); -} - - -// TODO(dcarney): remove this function when TurboFan supports it. -// Takes (the object to be iterated over, -// cache_array from ForInInit, -// cache_type from ForInInit, -// the current index) -// Returns pair (array[index], needs_filtering). -RUNTIME_FUNCTION_RETURN_PAIR(Runtime_ForInNext) { - SealHandleScope scope(isolate); - DCHECK(args.length() == 4); - int32_t index; - // This simulates CONVERT_ARG_HANDLE_CHECKED for calls returning pairs. - // Not worth creating a macro atm as this function should be removed. - if (!args[0]->IsJSReceiver() || !args[1]->IsFixedArray() || - !args[2]->IsObject() || !args[3]->ToInt32(&index)) { - Object* error = isolate->ThrowIllegalOperation(); - return MakePair(error, isolate->heap()->undefined_value()); - } - Handle object = args.at(0); - Handle array = args.at(1); - Handle cache_type = args.at(2); - // Figure out first if a slow check is needed for this object. - bool slow_check_needed = false; - if (cache_type->IsMap()) { - if (object->map() != Map::cast(*cache_type)) { - // Object transitioned. Need slow check. - slow_check_needed = true; - } - } else { - // No slow check needed for proxies. - slow_check_needed = Smi::cast(*cache_type)->value() == 1; - } - return MakePair(array->get(index), - isolate->heap()->ToBoolean(slow_check_needed)); -} - - RUNTIME_FUNCTION(Runtime_IsArray) { SealHandleScope shs(isolate); DCHECK(args.length() == 1); diff --git a/src/runtime/runtime-forin.cc b/src/runtime/runtime-forin.cc new file mode 100644 index 0000000..fadd297 --- /dev/null +++ b/src/runtime/runtime-forin.cc @@ -0,0 +1,67 @@ +// Copyright 2015 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. + +#include "src/arguments.h" +#include "src/runtime/runtime-utils.h" +#include "src/v8.h" + +namespace v8 { +namespace internal { + +RUNTIME_FUNCTION(Runtime_ForInDone) { + SealHandleScope scope(isolate); + DCHECK_EQ(2, args.length()); + CONVERT_SMI_ARG_CHECKED(index, 0); + CONVERT_SMI_ARG_CHECKED(length, 1); + DCHECK_LE(0, index); + DCHECK_LE(index, length); + return isolate->heap()->ToBoolean(index == length); +} + + +RUNTIME_FUNCTION(Runtime_ForInFilter) { + HandleScope scope(isolate); + DCHECK_EQ(2, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + // TODO(turbofan): Fast case for array indices. + Handle name = Runtime::ToName(isolate, key).ToHandleChecked(); + Maybe result = JSReceiver::HasProperty(receiver, name); + if (result.IsJust() && result.FromJust()) return *name; + return isolate->heap()->undefined_value(); +} + + +RUNTIME_FUNCTION(Runtime_ForInNext) { + HandleScope scope(isolate); + DCHECK_EQ(4, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0); + CONVERT_ARG_HANDLE_CHECKED(FixedArray, cache_array, 1); + CONVERT_ARG_HANDLE_CHECKED(Object, cache_type, 2); + CONVERT_SMI_ARG_CHECKED(index, 3); + Handle key = handle(cache_array->get(index), isolate); + // Don't need filtering if expected map still matches that of the receiver, + // and neither for proxies. + if (receiver->map() == *cache_type || *cache_type == Smi::FromInt(0)) { + return *key; + } + // TODO(turbofan): Fast case for array indices. + Handle name = Runtime::ToName(isolate, key).ToHandleChecked(); + Maybe result = JSReceiver::HasProperty(receiver, name); + if (result.IsJust() && result.FromJust()) return *name; + return isolate->heap()->undefined_value(); +} + + +RUNTIME_FUNCTION(Runtime_ForInStep) { + SealHandleScope scope(isolate); + DCHECK_EQ(1, args.length()); + CONVERT_SMI_ARG_CHECKED(index, 0); + DCHECK_LE(0, index); + DCHECK_LT(index, Smi::kMaxValue); + return Smi::FromInt(index + 1); +} + +} // namespace internal +} // namespace v8 diff --git a/src/runtime/runtime-utils.h b/src/runtime/runtime-utils.h index c44e402..3fe3297 100644 --- a/src/runtime/runtime-utils.h +++ b/src/runtime/runtime-utils.h @@ -5,6 +5,7 @@ #ifndef V8_RUNTIME_RUNTIME_UTILS_H_ #define V8_RUNTIME_RUNTIME_UTILS_H_ +#include "src/runtime/runtime.h" namespace v8 { namespace internal { diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index c0a8af5..ea5570e 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -29,28 +29,27 @@ namespace internal { // Entries have the form F(name, number of arguments, number of values): -#define FOR_EACH_INTRINSIC_ARRAY(F) \ - F(FinishArrayPrototypeSetup, 1, 1) \ - F(SpecialArrayFunctions, 0, 1) \ - F(TransitionElementsKind, 2, 1) \ - F(PushIfAbsent, 2, 1) \ - F(ArrayConcat, 1, 1) \ - F(RemoveArrayHoles, 2, 1) \ - F(MoveArrayContents, 2, 1) \ - F(EstimateNumberOfElements, 1, 1) \ - F(GetArrayKeys, 2, 1) \ - F(ArrayConstructor, -1, 1) \ - F(ArrayConstructorWithSubclassing, -1, 1) \ - F(InternalArrayConstructor, -1, 1) \ - F(NormalizeElements, 1, 1) \ - F(GrowArrayElements, 2, 1) \ - F(HasComplexElements, 1, 1) \ - F(ForInCacheArrayLength, 2, 1) /* TODO(turbofan): Only temporary */ \ - F(IsArray, 1, 1) \ - F(HasCachedArrayIndex, 1, 1) \ - F(GetCachedArrayIndex, 1, 1) \ - F(FixedArrayGet, 2, 1) \ - F(FixedArraySet, 3, 1) \ +#define FOR_EACH_INTRINSIC_ARRAY(F) \ + F(FinishArrayPrototypeSetup, 1, 1) \ + F(SpecialArrayFunctions, 0, 1) \ + F(TransitionElementsKind, 2, 1) \ + F(PushIfAbsent, 2, 1) \ + F(ArrayConcat, 1, 1) \ + F(RemoveArrayHoles, 2, 1) \ + F(MoveArrayContents, 2, 1) \ + F(EstimateNumberOfElements, 1, 1) \ + F(GetArrayKeys, 2, 1) \ + F(ArrayConstructor, -1, 1) \ + F(ArrayConstructorWithSubclassing, -1, 1) \ + F(InternalArrayConstructor, -1, 1) \ + F(NormalizeElements, 1, 1) \ + F(GrowArrayElements, 2, 1) \ + F(HasComplexElements, 1, 1) \ + F(IsArray, 1, 1) \ + F(HasCachedArrayIndex, 1, 1) \ + F(GetCachedArrayIndex, 1, 1) \ + F(FixedArrayGet, 2, 1) \ + F(FixedArraySet, 3, 1) \ F(FastOneByteArrayJoin, 2, 1) @@ -188,6 +187,13 @@ namespace internal { F(DebugBreakInOptimizedCode, 0, 1) +#define FOR_EACH_INTRINSIC_FORIN(F) \ + F(ForInDone, 2, 1) \ + F(ForInFilter, 2, 1) \ + F(ForInNext, 4, 1) \ + F(ForInStep, 1, 1) + + #define FOR_EACH_INTRINSIC_FUNCTION(F) \ F(IsSloppyModeFunction, 1, 1) \ F(FunctionGetName, 1, 1) \ @@ -672,12 +678,10 @@ namespace internal { F(URIUnescape, 1, 1) -#define FOR_EACH_INTRINSIC_RETURN_PAIR(F) \ - F(LoadLookupSlot, 2, 2) \ - F(LoadLookupSlotNoReferenceError, 2, 2) \ - F(ResolvePossiblyDirectEval, 6, 2) \ - F(ForInInit, 2, 2) /* TODO(turbofan): Only temporary */ \ - F(ForInNext, 4, 2) /* TODO(turbofan): Only temporary */ +#define FOR_EACH_INTRINSIC_RETURN_PAIR(F) \ + F(LoadLookupSlot, 2, 2) \ + F(LoadLookupSlotNoReferenceError, 2, 2) \ + F(ResolvePossiblyDirectEval, 6, 2) #define FOR_EACH_INTRINSIC_RETURN_OBJECT(F) \ @@ -687,6 +691,7 @@ namespace internal { FOR_EACH_INTRINSIC_COMPILER(F) \ FOR_EACH_INTRINSIC_DATE(F) \ FOR_EACH_INTRINSIC_DEBUG(F) \ + FOR_EACH_INTRINSIC_FORIN(F) \ FOR_EACH_INTRINSIC_FUNCTION(F) \ FOR_EACH_INTRINSIC_GENERATOR(F) \ FOR_EACH_INTRINSIC_I18N(F) \ diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 22bc23c..947de32 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -1231,9 +1231,9 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { // just skip it. __ Push(rcx); // Enumerable. __ Push(rbx); // Current entry. - __ InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION); + __ CallRuntime(Runtime::kForInFilter, 2); PrepareForBailoutForId(stmt->FilterId(), TOS_REG); - __ Cmp(rax, Smi::FromInt(0)); + __ CompareRoot(rax, Heap::kUndefinedValueRootIndex); __ j(equal, loop_statement.continue_label()); __ movp(rbx, rax); diff --git a/tools/gyp/v8.gyp b/tools/gyp/v8.gyp index 5bfcaf2..f2a8dd3 100644 --- a/tools/gyp/v8.gyp +++ b/tools/gyp/v8.gyp @@ -834,6 +834,7 @@ '../../src/runtime/runtime-compiler.cc', '../../src/runtime/runtime-date.cc', '../../src/runtime/runtime-debug.cc', + '../../src/runtime/runtime-forin.cc', '../../src/runtime/runtime-function.cc', '../../src/runtime/runtime-generator.cc', '../../src/runtime/runtime-i18n.cc',