[turbofan] Fix OSR compilations of for-in.

R=mstarzinger@chromium.org
LOG=Y
BUG=

Review URL: https://codereview.chromium.org/890543002

Cr-Commit-Position: refs/heads/master@{#26333}

src/compiler.cc

@@ -23,6 +23,7 @@
 #include "src/liveedit.h"
 #include "src/messages.h"
 #include "src/parser.h"
+#include "src/prettyprinter.h"
 #include "src/rewriter.h"
 #include "src/runtime-profiler.h"
 #include "src/scanner-character-streams.h"
@@ -412,7 +413,9 @@ OptimizedCompileJob::Status OptimizedCompileJob::CreateGraph() {
     if (FLAG_trace_opt) {
       OFStream os(stdout);
       os << "[compiling method " << Brief(*info()->closure())
-         << " using TurboFan]" << std::endl;
+         << " using TurboFan";
+      if (info()->is_osr()) os << " OSR";
+      os << "]" << std::endl;
     }
     Timer t(this, &time_taken_to_create_graph_);
     compiler::Pipeline pipeline(info());
@@ -425,7 +428,9 @@ OptimizedCompileJob::Status OptimizedCompileJob::CreateGraph() {
   if (FLAG_trace_opt) {
     OFStream os(stdout);
     os << "[compiling method " << Brief(*info()->closure())
-       << " using Crankshaft]" << std::endl;
+       << " using Crankshaft";
+    if (info()->is_osr()) os << " OSR";
+    os << "]" << std::endl;
   }
 
   if (FLAG_trace_hydrogen) {
@@ -1589,4 +1594,11 @@ bool CompilationPhase::ShouldProduceTraceOutput() const {
       base::OS::StrChr(const_cast<char*>(FLAG_trace_phase), name_[0]) != NULL);
 }
 
+
+#if DEBUG
+void CompilationInfo::PrintAstForTesting() {
+  PrintF("--- Source from AST ---\n%s\n",
+         PrettyPrinter(isolate(), zone()).PrintProgram(function()));
+}
+#endif
 } }  // namespace v8::internal

src/compiler.h

@@ -405,6 +405,10 @@ class CompilationInfo {
     ast_value_factory_owned_ = owned;
   }
 
+#if DEBUG
+  void PrintAstForTesting();
+#endif
+
  protected:
   CompilationInfo(Handle<SharedFunctionInfo> shared_info,
                   Zone* zone);

src/compiler/ast-graph-builder.cc

@@ -702,90 +702,9 @@ void AstGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
         environment()->Push(cache_array);
         environment()->Push(cache_length);
         environment()->Push(jsgraph()->ZeroConstant());
-        // PrepareForBailoutForId(stmt->BodyId(), NO_REGISTERS);
-        LoopBuilder for_loop(this);
-        for_loop.BeginLoop(GetVariablesAssignedInLoop(stmt),
-                           IsOsrLoopEntry(stmt));
-        // Check loop termination condition.
-        Node* index = environment()->Peek(0);
-        Node* exit_cond =
-            NewNode(javascript()->LessThan(), index, cache_length);
-        // TODO(jarin): provide real bailout id.
-        PrepareFrameState(exit_cond, BailoutId::None());
-        for_loop.BreakUnless(exit_cond);
-        // TODO(dcarney): this runtime call should be a handful of
-        //                simplified instructions that
-        //                basically produce
-        //                    value = array[index]
-        environment()->Push(obj);
-        environment()->Push(cache_array);
-        environment()->Push(cache_type);
-        environment()->Push(index);
-        Node* pair = ProcessArguments(
-            javascript()->CallRuntime(Runtime::kForInNext, 4), 4);
-        Node* value = NewNode(common()->Projection(0), pair);
-        Node* should_filter = NewNode(common()->Projection(1), pair);
-        environment()->Push(value);
-        {
-          // Test if FILTER_KEY needs to be called.
-          IfBuilder test_should_filter(this);
-          Node* should_filter_cond =
-              NewNode(javascript()->StrictEqual(), should_filter,
-                      jsgraph()->TrueConstant());
-          test_should_filter.If(should_filter_cond);
-          test_should_filter.Then();
-          value = environment()->Pop();
-          Node* builtins = BuildLoadBuiltinsObject();
-          Node* function = BuildLoadObjectField(
-              builtins,
-              JSBuiltinsObject::OffsetOfFunctionWithId(Builtins::FILTER_KEY));
-          // Callee.
-          environment()->Push(function);
-          // Receiver.
-          environment()->Push(obj);
-          // Args.
-          environment()->Push(value);
-          // result is either the string key or Smi(0) indicating the property
-          // is gone.
-          Node* res = ProcessArguments(
-              javascript()->CallFunction(3, NO_CALL_FUNCTION_FLAGS), 3);
-          // TODO(jarin): provide real bailout id.
-          PrepareFrameState(res, BailoutId::None());
-          Node* property_missing = NewNode(javascript()->StrictEqual(), res,
-                                           jsgraph()->ZeroConstant());
-          {
-            IfBuilder is_property_missing(this);
-            is_property_missing.If(property_missing);
-            is_property_missing.Then();
-            // Inc counter and continue.
-            Node* index_inc =
-                NewNode(javascript()->Add(), index, jsgraph()->OneConstant());
-            // TODO(jarin): provide real bailout id.
-            PrepareFrameState(index_inc, BailoutId::None());
-            environment()->Poke(0, index_inc);
-            for_loop.Continue();
-            is_property_missing.Else();
-            is_property_missing.End();
-          }
-          // Replace 'value' in environment.
-          environment()->Push(res);
-          test_should_filter.Else();
-          test_should_filter.End();
-        }
-        value = environment()->Pop();
-        // Bind value and do loop body.
-        VisitForInAssignment(stmt->each(), value, stmt->AssignmentId());
-        VisitIterationBody(stmt, &for_loop, 5);
-        for_loop.EndBody();
-        // Inc counter and continue.
-        Node* index_inc =
-            NewNode(javascript()->Add(), index, jsgraph()->OneConstant());
-        // TODO(jarin): provide real bailout id.
-        PrepareFrameState(index_inc, BailoutId::None());
-        environment()->Poke(0, index_inc);
-        for_loop.EndLoop();
-        environment()->Drop(5);
-        // PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS);
+
+        // Build the actual loop body.
+        VisitForInBody(stmt);
       }
       have_no_properties.End();
     }
@@ -795,6 +714,91 @@ void AstGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
 }
 
 
+// TODO(dcarney): this is a big function.  Try to clean up some.
+void AstGraphBuilder::VisitForInBody(ForInStatement* stmt) {
+  LoopBuilder for_loop(this);
+  for_loop.BeginLoop(GetVariablesAssignedInLoop(stmt), IsOsrLoopEntry(stmt));
+
+  // These stack values are renamed in the case of OSR, so reload them
+  // from the environment.
+  Node* index = environment()->Peek(0);
+  Node* cache_length = environment()->Peek(1);
+  Node* cache_array = environment()->Peek(2);
+  Node* cache_type = environment()->Peek(3);
+  Node* obj = environment()->Peek(4);
+
+  // Check loop termination condition.
+  Node* exit_cond = NewNode(javascript()->LessThan(), index, cache_length);
+  // TODO(jarin): provide real bailout id.
+  PrepareFrameState(exit_cond, BailoutId::None());
+  for_loop.BreakUnless(exit_cond);
+  Node* pair = NewNode(javascript()->CallRuntime(Runtime::kForInNext, 4), obj,
+                       cache_array, cache_type, index);
+  Node* value = NewNode(common()->Projection(0), pair);
+  Node* should_filter = NewNode(common()->Projection(1), pair);
+  environment()->Push(value);
+  {
+    // Test if FILTER_KEY needs to be called.
+    IfBuilder test_should_filter(this);
+    Node* should_filter_cond = NewNode(
+        javascript()->StrictEqual(), should_filter, jsgraph()->TrueConstant());
+    test_should_filter.If(should_filter_cond);
+    test_should_filter.Then();
+    value = environment()->Pop();
+    Node* builtins = BuildLoadBuiltinsObject();
+    Node* function = BuildLoadObjectField(
+        builtins,
+        JSBuiltinsObject::OffsetOfFunctionWithId(Builtins::FILTER_KEY));
+    // Callee.
+    environment()->Push(function);
+    // Receiver.
+    environment()->Push(obj);
+    // Args.
+    environment()->Push(value);
+    // result is either the string key or Smi(0) indicating the property
+    // is gone.
+    Node* res = ProcessArguments(
+        javascript()->CallFunction(3, NO_CALL_FUNCTION_FLAGS), 3);
+    // TODO(jarin): provide real bailout id.
+    PrepareFrameState(res, BailoutId::None());
+    Node* property_missing =
+        NewNode(javascript()->StrictEqual(), res, jsgraph()->ZeroConstant());
+    {
+      IfBuilder is_property_missing(this);
+      is_property_missing.If(property_missing);
+      is_property_missing.Then();
+      // Inc counter and continue.
+      Node* index_inc =
+          NewNode(javascript()->Add(), index, jsgraph()->OneConstant());
+      // TODO(jarin): provide real bailout id.
+      PrepareFrameState(index_inc, BailoutId::None());
+      environment()->Poke(0, index_inc);
+      for_loop.Continue();
+      is_property_missing.Else();
+      is_property_missing.End();
+    }
+    // Replace 'value' in environment.
+    environment()->Push(res);
+    test_should_filter.Else();
+    test_should_filter.End();
+  }
+  value = environment()->Pop();
+  // Bind value and do loop body.
+  VisitForInAssignment(stmt->each(), value, stmt->AssignmentId());
+  VisitIterationBody(stmt, &for_loop, 5);
+  for_loop.EndBody();
+  // Inc counter and continue.
+  Node* index_inc =
+      NewNode(javascript()->Add(), index, jsgraph()->OneConstant());
+  // TODO(jarin): provide real bailout id.
+  PrepareFrameState(index_inc, BailoutId::None());
+  environment()->Poke(0, index_inc);
+  for_loop.EndLoop();
+  environment()->Drop(5);
+  // PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS);
+}
+
+
 void AstGraphBuilder::VisitForOfStatement(ForOfStatement* stmt) {
   LoopBuilder for_loop(this);
   VisitForEffect(stmt->assign_iterator());

src/compiler/ast-graph-builder.h

@@ -199,6 +199,7 @@ class AstGraphBuilder : public StructuredGraphBuilder, public AstVisitor {
   // Dispatched from VisitForInStatement.
   void VisitForInAssignment(Expression* expr, Node* value,
                             BailoutId bailout_id);
+  void VisitForInBody(ForInStatement* stmt);
 
   // Dispatched from VisitClassLiteral.
   void VisitClassLiteralContents(ClassLiteral* expr);

src/compiler/graph-builder.cc

@@ -201,16 +201,11 @@ void StructuredGraphBuilder::Environment::PrepareForLoop(BitVector* assigned,
 
     for (int i = 0; i < size; ++i) {
       Node* val = values()->at(i);
-      // TODO(titzer): use IrOpcode::IsConstant() or similar.
-      if (val->opcode() == IrOpcode::kNumberConstant ||
-          val->opcode() == IrOpcode::kInt32Constant ||
-          val->opcode() == IrOpcode::kInt64Constant ||
-          val->opcode() == IrOpcode::kFloat64Constant ||
-          val->opcode() == IrOpcode::kHeapConstant)
-        continue;
-      Node* osr_value =
-          graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry);
-      values()->at(i) = builder_->MergeValue(val, osr_value, control);
+      if (!IrOpcode::IsConstantOpcode(val->opcode())) {
+        Node* osr_value =
+            graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry);
+        values()->at(i) = builder_->MergeValue(val, osr_value, control);
+      }
     }
   }
 }

test/mjsunit/asm/int32modb.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var stdlib = {};
+var foreign = {};
+var heap = new ArrayBuffer(64 * 1024);
+
+var mod = (function Module(stdlib, foreign, heap) {
+  "use asm";
+  function mod(dividend, divisor) {
+    dividend = dividend|0;
+    divisor = divisor|0;
+    return (dividend % divisor) | 0;
+  }
+  return { mod: mod };
+})(stdlib, foreign, heap).mod;
+
+var divisors = [-2147483648, -32 * 1024, -1000, -16, -7, -2, -1, 0,
+                1, 3, 4, 10, 64, 99, 1023, 1024, 2147483647];
+for (var i = 0; i < divisors.length; i++) {
+  var divisor = divisors[i];
+  for (var dividend = -2147483648; dividend < 2147483648; dividend += 3999773) {
+    assertEquals((dividend % divisor) | 0, mod(dividend, divisor));
+  }
+}

test/mjsunit/compiler/osr-forin.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --use-osr --turbo-osr
+
+function f(a) {
+  var sum = 0;
+  for (var j in a) {
+    var i = a[j];
+    var x = i + 2;
+    var y = x + 5;
+    var z = y + 3;
+    sum += z;
+  }
+  return sum;
+}
+
+var a = new Array();
+for (var i = 0; i < 10000; i++) {
+  a[i] = (i * 999) % 77;
+}
+
+for (var i = 0; i < 3; i++) {
+  assertEquals(480270, f(a));
+}

test/mjsunit/compiler/osr-forof.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --use-osr --turbo-osr
+
+function f(a) {
+  var sum = 0;
+  for (var i of a) {
+    var x = i + 2;
+    var y = x + 5;
+    var z = y + 3;
+    sum += z;
+  }
+  return sum;
+}
+
+var a = new Array();
+for (var i = 0; i < 10000; i++) {
+  a[i] = (i * 999) % 77;
+}
+
+for (var i = 0; i < 3; i++) {
+  assertEquals(480270, f(wrap(a)));
+}
+
+function wrap(array) {
+  var iterable = {};
+  var i = 0;
+  function next() {
+    return { done: i >= array.length, value: array[i++] };
+  };
+  iterable[Symbol.iterator] = function() { return { next:next }; };
+  return iterable;
+}