Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(469)

Unified Diff: src/compiler/js-inlining.cc

Issue 2333213002: [turbofan] Avoid unnecessary JSConvertReceiver nodes. (Closed)
Patch Set: Remove StoreField handling. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/compiler/js-inlining.cc
diff --git a/src/compiler/js-inlining.cc b/src/compiler/js-inlining.cc
index 721dfa97eca734d2c121a72e55464cf084634fbd..db7490e2235ed8f4d815968bd64a1686d6b12e41 100644
--- a/src/compiler/js-inlining.cc
+++ b/src/compiler/js-inlining.cc
@@ -275,6 +275,56 @@ Node* JSInliner::CreateTailCallerFrameState(Node* node, Node* 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) {
+ for (Node* dominator = effect;;) {
+ if (dominator->opcode() == IrOpcode::kCheckMaps &&
+ dominator->InputAt(0) == receiver) {
+ // Check if all maps have the given {instance_type}.
+ for (int i = 1; i < dominator->op()->ValueInputCount(); ++i) {
+ HeapObjectMatcher m(NodeProperties::GetValueInput(dominator, i));
+ if (!m.HasValue()) return true;
+ Handle<Map> const map = Handle<Map>::cast(m.Value());
+ if (!map->IsJSReceiverMap()) return true;
+ }
+ return false;
+ }
+ switch (dominator->opcode()) {
+ case IrOpcode::kStoreField: {
+ FieldAccess const& access = FieldAccessOf(dominator->op());
+ if (access.base_is_tagged == kTaggedBase &&
+ access.offset == HeapObject::kMapOffset) {
+ return true;
+ }
+ break;
+ }
+ case IrOpcode::kStoreElement:
+ case IrOpcode::kStoreTypedElement:
+ break;
+ default: {
+ DCHECK_EQ(1, dominator->op()->EffectOutputCount());
+ if (dominator->op()->EffectInputCount() != 1 ||
+ !dominator->op()->HasProperty(Operator::kNoWrite)) {
+ // Didn't find any appropriate CheckMaps node.
+ return true;
+ }
+ break;
+ }
+ }
+ dominator = NodeProperties::GetEffectInput(dominator);
+ }
+}
+
// TODO(mstarzinger,verwaest): Move this predicate onto SharedFunctionInfo?
bool NeedsImplicitReceiver(Handle<SharedFunctionInfo> shared_info) {
DisallowHeapAllocation no_gc;
@@ -561,14 +611,16 @@ Reduction JSInliner::ReduceJSCall(Node* node, Handle<JSFunction> function) {
// any number of times, it's not observable.
if (node->opcode() == IrOpcode::kJSCallFunction &&
is_sloppy(parse_info.language_mode()) && !shared_info->native()) {
- const CallFunctionParameters& p = CallFunctionParametersOf(node->op());
- Node* frame_state_before = NodeProperties::FindFrameStateBefore(node);
Node* effect = NodeProperties::GetEffectInput(node);
- Node* convert = graph()->NewNode(
- javascript()->ConvertReceiver(p.convert_mode()), call.receiver(),
- context, frame_state_before, effect, start);
- NodeProperties::ReplaceValueInput(node, convert, 1);
- NodeProperties::ReplaceEffectInput(node, convert);
+ if (NeedsConvertReceiver(call.receiver(), effect)) {
+ const CallFunctionParameters& p = CallFunctionParametersOf(node->op());
+ Node* frame_state_before = NodeProperties::FindFrameStateBefore(node);
+ Node* convert = effect = graph()->NewNode(
+ javascript()->ConvertReceiver(p.convert_mode()), call.receiver(),
+ context, frame_state_before, effect, start);
+ NodeProperties::ReplaceValueInput(node, convert, 1);
+ NodeProperties::ReplaceEffectInput(node, effect);
+ }
}
// If we are inlining a JS call at tail position then we have to pop current
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698