Commit 947101c8 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[turbofan] Properly optimize polymorphic constructor call inlining.

This CL addresses a couple of minor issues that were in the way of
properly inlining polymorphic constructors calls, i.e. as found in
this common pattern using Symbol.species:

  class A {
    static get [Symbol.species]() { return this; }
    clone() { return new this.constructor[Symbol.species](); }
  }
  class B extends A {
    static get [Symbol.species]() { return this; }
  }

  function foo(o) { return o.clone(); }
  foo(new A());
  foo(new B());

Here the call to this.constructor[Symbol.species]() is the interesting
site. To get this fully inlined, we had to

  - make sure we don't introduce too many CheckHeapObject eagerly that
    block later optimizations (instead we try harder to see whether the
    receiver is already provably a HeapObject), and
  - also update the new.target of polymorphic JSConstruct nodes, when
    it refers to the same node as the target that we're specializing
    to (this way the JSCreate becomes fully inlinable later).

This seems to yield a solid 1.5% on the ARES6 ML benchmark (run via the
d8 cli runner), which confirms the previous profiled estimation. On the
micro-benchmark that specifically measures this feature in isolation we
go from

  testClone: 828 ms.

on V8 ToT as of today and

  testClone: 1439 ms.

on V8 6.1 to

  testClone: 219 ms.

which is a 3.7x improvement, on top of the previous ~2x boost that we
got from inlining the polymorphic symbol lookup.

Bug: v8:6885, v8:6278, v8:6344
Change-Id: Ida7abf683c7879978f181ba7f52a125f4f83ae6f
Reviewed-on: https://chromium-review.googlesource.com/700596Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48284}
parent 808dc8cf
......@@ -600,6 +600,12 @@ void JSInliningHeuristic::CreateOrReuseDispatch(Node* node, Node* callee,
// Clone the calls for each branch.
// The first input to the call is the actual target (which we specialize
// to the known {target}); the last input is the control dependency.
// We also specialize the new.target of JSConstruct {node}s if it refers
// to the same node as the {node}'s target input, so that we can later
// properly inline the JSCreate operations.
if (node->opcode() == IrOpcode::kJSConstruct && inputs[0] == inputs[1]) {
inputs[1] = target;
}
inputs[0] = target;
inputs[input_count - 1] = if_successes[i];
calls[i] = if_successes[i] =
......
......@@ -89,8 +89,9 @@ bool PropertyAccessBuilder::TryBuildNumberCheck(MapHandles const& maps,
return false;
}
Node* PropertyAccessBuilder::BuildCheckHeapObject(Node* receiver, Node** effect,
Node* control) {
namespace {
bool NeedsCheckHeapObject(Node* receiver) {
switch (receiver->opcode()) {
case IrOpcode::kHeapConstant:
case IrOpcode::kJSCreate:
......@@ -99,22 +100,44 @@ Node* PropertyAccessBuilder::BuildCheckHeapObject(Node* receiver, Node** effect,
case IrOpcode::kJSCreateClosure:
case IrOpcode::kJSCreateIterResultObject:
case IrOpcode::kJSCreateLiteralArray:
case IrOpcode::kJSCreateEmptyLiteralArray:
case IrOpcode::kJSCreateLiteralObject:
case IrOpcode::kJSCreateEmptyLiteralObject:
case IrOpcode::kJSCreateLiteralRegExp:
case IrOpcode::kJSCreateGeneratorObject:
case IrOpcode::kJSConvertReceiver:
case IrOpcode::kJSConstructForwardVarargs:
case IrOpcode::kJSConstruct:
case IrOpcode::kJSConstructWithArrayLike:
case IrOpcode::kJSConstructWithSpread:
case IrOpcode::kJSToName:
case IrOpcode::kJSToString:
case IrOpcode::kJSToObject:
case IrOpcode::kJSTypeOf: {
return receiver;
}
default: {
return *effect = graph()->NewNode(simplified()->CheckHeapObject(),
receiver, *effect, control);
case IrOpcode::kJSTypeOf:
case IrOpcode::kJSGetSuperConstructor:
return false;
case IrOpcode::kPhi: {
Node* control = NodeProperties::GetControlInput(receiver);
if (control->opcode() != IrOpcode::kMerge) return true;
for (int i = 0; i < receiver->InputCount() - 1; ++i) {
if (NeedsCheckHeapObject(receiver->InputAt(i))) return true;
}
return false;
}
default:
return true;
}
UNREACHABLE();
return nullptr;
}
} // namespace
Node* PropertyAccessBuilder::BuildCheckHeapObject(Node* receiver, Node** effect,
Node* control) {
if (NeedsCheckHeapObject(receiver)) {
receiver = *effect = graph()->NewNode(simplified()->CheckHeapObject(),
receiver, *effect, control);
}
return receiver;
}
void PropertyAccessBuilder::BuildCheckMaps(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment