[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 <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49695}
This commit is contained in:
Benedikt Meurer 2017-11-28 19:55:18 +01:00 committed by Commit Bot
parent 2f22fca9a0
commit 9fb39c6b91
5 changed files with 101 additions and 129 deletions

View File

@ -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<HeapObject> 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<HeapObject> 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(

View File

@ -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<Map> 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<SharedFunctionInfo> 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()));

View File

@ -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<HeapObject> 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<Map> 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<HeapObject> 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);

View File

@ -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.

View File

@ -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) \