From 9fb39c6b918ba71b126eecd9e538c28d8bb4b7da Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Tue, 28 Nov 2017 19:55:18 +0100 Subject: [PATCH] [turbofan] Unify CanBePrimitive and NeedsConvertReceiver. The two helper functions CanBePrimitive and NeedsConvertReceiver did essentially the same, just in a slightly different way, and both weren't really robust wrt. to the list of JSConstruct* and JSCreate* operators that they were handling. There's now a single helper in the NodeProperties and a couple of extra macro lists to keep this list up to date more easily. Drive-by-fix: Also moved the CanBeNullOrUndefined helper to the NodeProperties class. Bug: v8:5267, v8:7109 Change-Id: Ibbf387040e3f424ee224c53fac15c2b3207b1926 Reviewed-on: https://chromium-review.googlesource.com/793734 Reviewed-by: Yang Guo Commit-Queue: Benedikt Meurer Cr-Commit-Position: refs/heads/master@{#49695} --- src/compiler/js-call-reducer.cc | 67 +++------------------------------ src/compiler/js-inlining.cc | 51 +------------------------ src/compiler/node-properties.cc | 63 +++++++++++++++++++++++++++++++ src/compiler/node-properties.h | 9 +++++ src/compiler/opcodes.h | 40 +++++++++++--------- 5 files changed, 101 insertions(+), 129 deletions(-) diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index 54d2341b77..a7702d830a 100644 --- a/src/compiler/js-call-reducer.cc +++ b/src/compiler/js-call-reducer.cc @@ -22,64 +22,6 @@ namespace v8 { namespace internal { namespace compiler { -namespace { - -bool CanBePrimitive(Node* node) { - switch (node->opcode()) { - case IrOpcode::kConvertReceiver: - case IrOpcode::kJSCreate: - case IrOpcode::kJSCreateArguments: - case IrOpcode::kJSCreateArray: - case IrOpcode::kJSCreateBoundFunction: - case IrOpcode::kJSCreateClosure: - case IrOpcode::kJSCreateEmptyLiteralArray: - case IrOpcode::kJSCreateEmptyLiteralObject: - case IrOpcode::kJSCreateIterResultObject: - case IrOpcode::kJSCreateKeyValueArray: - case IrOpcode::kJSCreateLiteralArray: - case IrOpcode::kJSCreateLiteralObject: - case IrOpcode::kJSCreateLiteralRegExp: - case IrOpcode::kJSConstructForwardVarargs: - case IrOpcode::kJSConstruct: - case IrOpcode::kJSConstructWithArrayLike: - case IrOpcode::kJSConstructWithSpread: - case IrOpcode::kJSGetSuperConstructor: - case IrOpcode::kJSToObject: - return false; - case IrOpcode::kHeapConstant: { - Handle value = HeapObjectMatcher(node).Value(); - return value->IsPrimitive(); - } - default: - return true; - } -} - -bool CanBeNullOrUndefined(Node* node) { - if (CanBePrimitive(node)) { - switch (node->opcode()) { - case IrOpcode::kToBoolean: - case IrOpcode::kJSToInteger: - case IrOpcode::kJSToLength: - case IrOpcode::kJSToName: - case IrOpcode::kJSToNumber: - case IrOpcode::kJSToNumeric: - case IrOpcode::kJSToString: - return false; - case IrOpcode::kHeapConstant: { - Handle value = HeapObjectMatcher(node).Value(); - Isolate* const isolate = value->GetIsolate(); - return value->IsNullOrUndefined(isolate); - } - default: - return true; - } - } - return false; -} - -} // namespace - Reduction JSCallReducer::Reduce(Node* node) { switch (node->opcode()) { case IrOpcode::kJSConstruct: @@ -169,10 +111,11 @@ Reduction JSCallReducer::ReduceObjectConstructor(Node* node) { if (p.arity() < 3) return NoChange(); Node* value = (p.arity() >= 3) ? NodeProperties::GetValueInput(node, 2) : jsgraph()->UndefinedConstant(); + Node* effect = NodeProperties::GetEffectInput(node); // We can fold away the Object(x) call if |x| is definitely not a primitive. - if (CanBePrimitive(value)) { - if (!CanBeNullOrUndefined(value)) { + if (NodeProperties::CanBePrimitive(value, effect)) { + if (!NodeProperties::CanBeNullOrUndefined(value, effect)) { // Turn the {node} into a {JSToObject} call if we know that // the {value} cannot be null or undefined. NodeProperties::ReplaceValueInputs(node, value); @@ -213,7 +156,7 @@ Reduction JSCallReducer::ReduceFunctionPrototypeApply(Node* node) { // If {arguments_list} cannot be null or undefined, we don't need // to expand this {node} to control-flow. - if (!CanBeNullOrUndefined(arguments_list)) { + if (!NodeProperties::CanBeNullOrUndefined(arguments_list, effect)) { // Massage the value inputs appropriately. node->ReplaceInput(0, target); node->ReplaceInput(1, this_argument); @@ -2063,7 +2006,7 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) { // Update the JSCall operator on {node}. ConvertReceiverMode const convert_mode = - CanBeNullOrUndefined(bound_this) + NodeProperties::CanBeNullOrUndefined(bound_this, effect) ? ConvertReceiverMode::kAny : ConvertReceiverMode::kNotNullOrUndefined; NodeProperties::ChangeOp( diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc index 02f1d25d43..add2b2c478 100644 --- a/src/compiler/js-inlining.cc +++ b/src/compiler/js-inlining.cc @@ -258,55 +258,6 @@ Node* JSInliner::CreateArtificialFrameState(Node* node, Node* outer_frame_state, namespace { -// TODO(bmeurer): Unify this with the witness helper functions in the -// js-builtin-reducer.cc once we have a better understanding of the -// map tracking we want to do, and eventually changed the CheckMaps -// operator to carry map constants on the operator instead of inputs. -// I.e. if the CheckMaps has some kind of SmallMapSet as operator -// parameter, then this could be changed to call a generic -// -// SmallMapSet NodeProperties::CollectMapWitness(receiver, effect) -// -// function, which either returns the map set from the CheckMaps or -// a singleton set from a StoreField. -bool NeedsConvertReceiver(Node* receiver, Node* effect) { - // Check if the {receiver} is already a JSReceiver. - switch (receiver->opcode()) { - case IrOpcode::kConvertReceiver: - case IrOpcode::kJSConstruct: - case IrOpcode::kJSConstructWithSpread: - case IrOpcode::kJSCreate: - case IrOpcode::kJSCreateArguments: - case IrOpcode::kJSCreateArray: - case IrOpcode::kJSCreateBoundFunction: - case IrOpcode::kJSCreateClosure: - case IrOpcode::kJSCreateIterResultObject: - case IrOpcode::kJSCreateKeyValueArray: - case IrOpcode::kJSCreateLiteralArray: - case IrOpcode::kJSCreateLiteralObject: - case IrOpcode::kJSCreateLiteralRegExp: - case IrOpcode::kJSGetSuperConstructor: - case IrOpcode::kJSToObject: { - return false; - } - default: { - // We don't really care about the exact maps here, just the instance - // types, which don't change across potential side-effecting operations. - ZoneHandleSet maps; - NodeProperties::InferReceiverMapsResult result = - NodeProperties::InferReceiverMaps(receiver, effect, &maps); - if (result != NodeProperties::kNoReceiverMaps) { - // Check if all {maps} are actually JSReceiver maps. - for (size_t i = 0; i < maps.size(); ++i) { - if (!maps[i]->IsJSReceiverMap()) return true; - } - return false; - } - return true; - } - } -} - // TODO(mstarzinger,verwaest): Move this predicate onto SharedFunctionInfo? bool NeedsImplicitReceiver(Handle shared_info) { DisallowHeapAllocation no_gc; @@ -708,7 +659,7 @@ Reduction JSInliner::ReduceJSCall(Node* node) { if (node->opcode() == IrOpcode::kJSCall && is_sloppy(shared_info->language_mode()) && !shared_info->native()) { Node* effect = NodeProperties::GetEffectInput(node); - if (NeedsConvertReceiver(call.receiver(), effect)) { + if (NodeProperties::CanBePrimitive(call.receiver(), effect)) { CallParameters const& p = CallParametersOf(node->op()); Node* global_proxy = jsgraph()->HeapConstant( handle(info_->native_context()->global_proxy())); diff --git a/src/compiler/node-properties.cc b/src/compiler/node-properties.cc index 9fc19fdad2..51225bf886 100644 --- a/src/compiler/node-properties.cc +++ b/src/compiler/node-properties.cc @@ -500,6 +500,69 @@ bool NodeProperties::NoObservableSideEffectBetween(Node* effect, return true; } +// static +bool NodeProperties::CanBePrimitive(Node* receiver, Node* effect) { + switch (receiver->opcode()) { +#define CASE(Opcode) case IrOpcode::k##Opcode: + JS_CONSTRUCT_OP_LIST(CASE) + JS_CREATE_OP_LIST(CASE) +#undef CASE + case IrOpcode::kCheckReceiver: + case IrOpcode::kConvertReceiver: + case IrOpcode::kJSGetSuperConstructor: + case IrOpcode::kJSToObject: + return false; + case IrOpcode::kHeapConstant: { + Handle value = HeapObjectMatcher(receiver).Value(); + return value->IsPrimitive(); + } + default: { + // We don't really care about the exact maps here, + // just the instance types, which don't change + // across potential side-effecting operations. + ZoneHandleSet maps; + if (InferReceiverMaps(receiver, effect, &maps) != kNoReceiverMaps) { + // Check if all {maps} are actually JSReceiver maps. + for (size_t i = 0; i < maps.size(); ++i) { + if (!maps[i]->IsJSReceiverMap()) return true; + } + return false; + } + return true; + } + } +} + +// static +bool NodeProperties::CanBeNullOrUndefined(Node* receiver, Node* effect) { + if (CanBePrimitive(receiver, effect)) { + switch (receiver->opcode()) { + case IrOpcode::kCheckSmi: + case IrOpcode::kCheckNumber: + case IrOpcode::kCheckSymbol: + case IrOpcode::kCheckString: + case IrOpcode::kCheckSeqString: + case IrOpcode::kCheckInternalizedString: + case IrOpcode::kToBoolean: + case IrOpcode::kJSToInteger: + case IrOpcode::kJSToLength: + case IrOpcode::kJSToName: + case IrOpcode::kJSToNumber: + case IrOpcode::kJSToNumeric: + case IrOpcode::kJSToString: + return false; + case IrOpcode::kHeapConstant: { + Handle value = HeapObjectMatcher(receiver).Value(); + Isolate* const isolate = value->GetIsolate(); + return value->IsNullOrUndefined(isolate); + } + default: + return true; + } + } + return false; +} + // static Node* NodeProperties::GetOuterContext(Node* node, size_t* depth) { Node* context = NodeProperties::GetContextInput(node); diff --git a/src/compiler/node-properties.h b/src/compiler/node-properties.h index f28085d70a..5ccc15c1ab 100644 --- a/src/compiler/node-properties.h +++ b/src/compiler/node-properties.h @@ -158,6 +158,15 @@ class V8_EXPORT_PRIVATE NodeProperties final { // in the effect chain. static bool NoObservableSideEffectBetween(Node* effect, Node* dominator); + // Returns true if the {receiver} can be a primitive value (i.e. is not + // definitely a JavaScript object); might walk up the {effect} chain to + // find map checks on {receiver}. + static bool CanBePrimitive(Node* receiver, Node* effect); + + // Returns true if the {receiver} can be null or undefined. Might walk + // up the {effect} chain to find map checks for {receiver}. + static bool CanBeNullOrUndefined(Node* receiver, Node* effect); + // --------------------------------------------------------------------------- // Context. diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index 651635db18..0827ca12b1 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -131,19 +131,23 @@ V(JSIncrement) \ V(JSNegate) +#define JS_CREATE_OP_LIST(V) \ + V(JSCreate) \ + V(JSCreateArguments) \ + V(JSCreateArray) \ + V(JSCreateBoundFunction) \ + V(JSCreateClosure) \ + V(JSCreateGeneratorObject) \ + V(JSCreateIterResultObject) \ + V(JSCreateKeyValueArray) \ + V(JSCreateLiteralArray) \ + V(JSCreateEmptyLiteralArray) \ + V(JSCreateLiteralObject) \ + V(JSCreateEmptyLiteralObject) \ + V(JSCreateLiteralRegExp) + #define JS_OBJECT_OP_LIST(V) \ - V(JSCreate) \ - V(JSCreateArguments) \ - V(JSCreateArray) \ - V(JSCreateBoundFunction) \ - V(JSCreateClosure) \ - V(JSCreateIterResultObject) \ - V(JSCreateKeyValueArray) \ - V(JSCreateLiteralArray) \ - V(JSCreateEmptyLiteralArray) \ - V(JSCreateLiteralObject) \ - V(JSCreateEmptyLiteralObject) \ - V(JSCreateLiteralRegExp) \ + JS_CREATE_OP_LIST(V) \ V(JSLoadProperty) \ V(JSLoadNamed) \ V(JSLoadGlobal) \ @@ -154,7 +158,6 @@ V(JSStoreDataPropertyInLiteral) \ V(JSDeleteProperty) \ V(JSHasProperty) \ - V(JSCreateGeneratorObject) \ V(JSGetSuperConstructor) #define JS_CONTEXT_OP_LIST(V) \ @@ -165,11 +168,14 @@ V(JSCreateWithContext) \ V(JSCreateBlockContext) +#define JS_CONSTRUCT_OP_LIST(V) \ + V(JSConstructForwardVarargs) \ + V(JSConstruct) \ + V(JSConstructWithArrayLike) \ + V(JSConstructWithSpread) + #define JS_OTHER_OP_LIST(V) \ - V(JSConstructForwardVarargs) \ - V(JSConstruct) \ - V(JSConstructWithArrayLike) \ - V(JSConstructWithSpread) \ + JS_CONSTRUCT_OP_LIST(V) \ V(JSCallForwardVarargs) \ V(JSCall) \ V(JSCallWithArrayLike) \