Inline Array.prototype.forEach in TurboFan

src/compiler/js-call-reducer.cc

Index: src/compiler/js-call-reducer.cc
diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc
index 1e1d3a92ab01abc235b58dbec165db4ad3d83ecb..9a9c1e282bafeaedc36f4d62facd232538cf6d01 100644
--- a/src/compiler/js-call-reducer.cc
+++ b/src/compiler/js-call-reducer.cc
@@ -7,6 +7,7 @@
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
 #include "src/compilation-dependencies.h"
+#include "src/compiler/access-builder.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/linkage.h"
 #include "src/compiler/node-matchers.h"
@@ -350,6 +351,134 @@ Reduction JSCallReducer::ReduceReflectGetPrototypeOf(Node* node) {
   return ReduceObjectGetPrototype(node, target);
 }
 
+Reduction JSCallReducer::ReduceArrayForEach(Handle<SharedFunctionInfo> shared,
+                                            Node* node) {
+  if (!FLAG_inline_array_builtins) return NoChange();
+  DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
+  Node* outer_frame_state = NodeProperties::GetFrameStateInput(node);
+  Node* effect = NodeProperties::GetEffectInput(node);
+  Node* control = NodeProperties::GetControlInput(node);
+  Node* context = NodeProperties::GetContextInput(node);
+  CallParameters const& p = CallParametersOf(node->op());
+
+  // Try to determine the {receiver} map.
+  Node* receiver = NodeProperties::GetValueInput(node, 1);
+  Node* fncallback = node->op()->ValueInputCount() > 2
+                         ? NodeProperties::GetValueInput(node, 2)
+                         : jsgraph()->UndefinedConstant();
+  Node* this_arg = node->op()->ValueInputCount() > 3
+                       ? NodeProperties::GetValueInput(node, 3)
+                       : jsgraph()->UndefinedConstant();
+  ZoneHandleSet<Map> receiver_maps;
+  if (!NodeProperties::InferReceiverMaps(receiver, effect, &receiver_maps)) {

[Benedikt Meurer, 2017/05/22 19:29:40]

This returns an enum. You only want to deal with reliable receiver maps for now.

[danno, 2017/06/06 12:04:52]

Done.

+    return NoChange();
+  }
+  if (receiver_maps.size() != 1) return NoChange();
+  Handle<Map> receiver_map(receiver_maps[0]);
+  ElementsKind kind = receiver_map->elements_kind();
+  // TODO(danno): Handle holey Smi and Object fast elements kinds and double
+  // packed.
+  if (!IsFastPackedElementsKind(kind) || IsFastDoubleElementsKind(kind)) {
+    return NoChange();
+  }
+
+  Node* k = jsgraph()->Constant(0);

[Jarin, 2017/05/24 06:41:22]

a.k.a. ZeroConstant()

[danno, 2017/06/06 12:04:52]

Done.

+
+  Node* original_length = graph()->NewNode(
+      simplified()->LoadField(AccessBuilder::ForJSArrayLength(FAST_ELEMENTS)),
+      receiver, effect, control);
+
+  Node* loop = control = graph()->NewNode(common()->Loop(2), control, control);
+  Node* eloop = effect =
+      graph()->NewNode(common()->EffectPhi(2), effect, effect, loop);
+  Node* vloop = k = graph()->NewNode(
+      common()->Phi(MachineRepresentation::kTagged, 2), k, k, loop);
+
+  control = loop;
+  effect = eloop;
+
+  Node* continue_test =
+      graph()->NewNode(simplified()->NumberLessThan(), k, original_length);
+  Node* continue_branch = graph()->NewNode(common()->Branch(BranchHint::kTrue),
+                                           continue_test, control);
+
+  Node* if_true = graph()->NewNode(common()->IfTrue(), continue_branch);
+  Node* if_false = graph()->NewNode(common()->IfFalse(), continue_branch);
+  control = if_true;
+
+  std::vector<Node*> checkpoint_params(
+      {receiver, fncallback, this_arg, k, original_length});
+  const int stack_parameters = static_cast<int>(checkpoint_params.size());
+
+  Node* frame_state = CreateJavaScriptBuiltinContinuationFrameState(
+      jsgraph(), shared, Builtins::kArrayForEachLoopEagerDeoptContinuation,
+      node->InputAt(0), context, &checkpoint_params[0], stack_parameters,
+      outer_frame_state, ContinuationFrameStateMode::EAGER);
+
+  effect =
+      graph()->NewNode(common()->Checkpoint(), frame_state, effect, control);
+
+  // Make sure the map hasn't changed during the iteration
+  Node* orig_map = jsgraph()->HeapConstant(receiver_map);
+  Node* array_map = effect =
+      graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
+                       receiver, effect, control);
+  Node* check_map =
+      graph()->NewNode(simplified()->ReferenceEqual(), array_map, orig_map);
+  effect =
+      graph()->NewNode(simplified()->CheckIf(), check_map, effect, control);
+
+  // Make sure that the access is still in bounds, since the callback could have
+  // changed the array's size.
+  Node* length = graph()->NewNode(
+      simplified()->LoadField(AccessBuilder::ForJSArrayLength(FAST_ELEMENTS)),
+      receiver, effect, control);
+  k = effect =
+      graph()->NewNode(simplified()->CheckBounds(), k, length, effect, control);
+  USE(length);

[Benedikt Meurer, 2017/05/22 19:29:40]

This is unnecessary.

[danno, 2017/06/06 12:04:52]

Done.

+
+  // Reload the elements pointer before calling the callback, since the previous
+  // callback might have resized the array causing the elements buffer to be
+  // re-allocated.
+  Node* elements = graph()->NewNode(
+      simplified()->LoadField(AccessBuilder::ForJSObjectElements()), receiver,
+      effect, control);
+
+  Node* element = graph()->NewNode(
+      simplified()->LoadElement(AccessBuilder::ForFixedArrayElement()),
+      elements, k, effect, control);
+
+  Node* next_k =
+      graph()->NewNode(simplified()->NumberAdd(), k, jsgraph()->Constant(1));
+  checkpoint_params[3] = next_k;
+  frame_state = CreateJavaScriptBuiltinContinuationFrameState(
+      jsgraph(), shared, Builtins::kArrayForEachLoopLazyDeoptContinuation,
+      node->InputAt(0), context, &checkpoint_params[0], stack_parameters,
+      outer_frame_state, ContinuationFrameStateMode::LAZY);
+
+  Node* call_back = control = effect = graph()->NewNode(
+      javascript()->Call(5, p.frequency()), fncallback, this_arg, element, k,

[Benedikt Meurer, 2017/05/22 19:29:40]

This doesn't properly rewire IfException edges. Can you add an mjsunit test for
this? And either implement the rewiring or disable this optimization if node has
exception projection.

[danno, 2017/06/06 12:04:52]

Done.

+      receiver, context, frame_state, effect, control);
+  USE(call_back);

[Benedikt Meurer, 2017/05/22 19:29:40]

You don't seem to need the call_back variable.

[danno, 2017/06/06 12:04:52]

Done.

+  USE(this_arg);

[Benedikt Meurer, 2017/05/22 19:29:40]

This is unnecessary.

[danno, 2017/06/06 12:04:52]

Done.

+
+  k = next_k;
+
+  loop->ReplaceInput(1, control);
+  vloop->ReplaceInput(1, k);
+  eloop->ReplaceInput(1, effect);
+
+  control = if_false;
+  effect = eloop;
+
+  NodeProperties::ReplaceUses(node, jsgraph()->UndefinedConstant(), effect,

[Benedikt Meurer, 2017/05/22 19:29:40]

Can you use ReplaceWithValue instead of this magic (and the lines below)?

[Michael Starzinger, 2017/05/24 13:54:59]

+1, otherwise we might run into similar issues as here:
https://chromium-review.googlesource.com/508712

[danno, 2017/06/06 12:04:52]

Done.

[danno, 2017/06/06 12:04:52]

Done.

+                              control);
+
+  node->TrimInputCount(0);
+  NodeProperties::ChangeOp(node, common()->Dead());
+  return Changed(node);
+}
+
 Reduction JSCallReducer::ReduceCallApiFunction(
     Node* node, Handle<FunctionTemplateInfo> function_template_info) {
   DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
@@ -616,6 +745,8 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
           return ReduceObjectPrototypeGetProto(node);
         case Builtins::kReflectGetPrototypeOf:
           return ReduceReflectGetPrototypeOf(node);
+        case Builtins::kArrayForEach:
+          return ReduceArrayForEach(shared, node);
         default:
           break;
       }