[type profile] Collect return types.

Collect type information of return values.

Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.

Instead of the variable names, pass the
source location and print the function name.

Add an integration test for --type-profile that checks
for crashes.

Remove type feedback for assignments for now as it convolutes the output.

************ Function with 2 return statements ********
function testFunction(param, flag) {
  // We want to test 2 different return positions in one function.
  if (flag) {
    var first_var = param;
    return first_var;
  }
  var second_var = param;
  return second_var;
}

testFunction({});
testFunction(123, true);
testFunction('hello');
testFunction(undefined);
*******************************************************

************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: undefined
*******************************************************

Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base class.

BUG=v8:5935

Review-Url: https://codereview.chromium.org/2755973002
Cr-Commit-Position: refs/heads/master@{#43956}

src/ast/ast-numbering.cc

@@ -33,6 +33,13 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
 
   bool Renumber(FunctionLiteral* node);
 
+  FeedbackSlot TypeProfileSlotForReturnValue() const {
+    if (collect_type_profile_) {
+      DCHECK(!type_profile_for_return_value_.IsInvalid());
+    }
+    return type_profile_for_return_value_;
+  }
+
  private:
 // AST node visitor interface.
 #define DEFINE_VISIT(type) void Visit##type(type* node);
@@ -104,6 +111,7 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
   BailoutReason dont_optimize_reason_;
   HandlerTable::CatchPrediction catch_prediction_;
   bool collect_type_profile_;
+  FeedbackSlot type_profile_for_return_value_;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
   DISALLOW_COPY_AND_ASSIGN(AstNumberingVisitor);
@@ -237,6 +245,8 @@ void AstNumberingVisitor::VisitReturnStatement(ReturnStatement* node) {
   IncrementNodeCount();
   Visit(node->expression());
 
+  node->SetTypeProfileSlot(TypeProfileSlotForReturnValue());
+
   DCHECK(!node->is_async_return() ||
          properties_.flags() & AstProperties::kMustUseIgnitionTurbo);
 }
@@ -691,6 +701,11 @@ bool AstNumberingVisitor::Renumber(FunctionLiteral* node) {
 
   LanguageModeScope language_mode_scope(this, node->language_mode());
 
+  if (collect_type_profile_) {
+    type_profile_for_return_value_ =
+        properties_.get_spec()->AddTypeProfileSlot();
+  }
+
   VisitDeclarations(scope->declarations());
   VisitStatements(node->body());
 

src/ast/ast.h

@@ -904,6 +904,15 @@ class ReturnStatement final : public JumpStatement {
   Type type() const { return TypeField::decode(bit_field_); }
   bool is_async_return() const { return type() == kAsyncReturn; }
 
+  FeedbackSlot TypeProfileSlot() const {
+    DCHECK(HasTypeProfileSlot());
+    return type_profile_slot_;
+  }
+
+  void SetTypeProfileSlot(FeedbackSlot slot) { type_profile_slot_ = slot; }
+
+  bool HasTypeProfileSlot() const { return !type_profile_slot_.IsInvalid(); }
+
  private:
   friend class AstNodeFactory;
 
@@ -912,6 +921,8 @@ class ReturnStatement final : public JumpStatement {
     bit_field_ |= TypeField::encode(type);
   }
 
+  FeedbackSlot type_profile_slot_;
+
   Expression* expression_;
 
   class TypeField

src/compiler/bytecode-graph-builder.cc

@@ -806,8 +806,8 @@ void BytecodeGraphBuilder::VisitStaDataPropertyInLiteral() {
 void BytecodeGraphBuilder::VisitCollectTypeProfile() {
   PrepareEagerCheckpoint();
 
-  Node* name =
-      environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
+  Node* position =
+      jsgraph()->Constant(bytecode_iterator().GetImmediateOperand(0));
   Node* value = environment()->LookupAccumulator();
   Node* index = jsgraph()->Constant(bytecode_iterator().GetIndexOperand(1));
 
@@ -815,7 +815,7 @@ void BytecodeGraphBuilder::VisitCollectTypeProfile() {
 
   const Operator* op = javascript()->CallRuntime(Runtime::kCollectTypeProfile);
 
-  Node* node = NewNode(op, name, value, vector, index);
+  Node* node = NewNode(op, position, value, vector, index);
   environment()->RecordAfterState(node, Environment::kAttachFrameState);
 }
 

src/feedback-vector.cc

@@ -1049,7 +1049,7 @@ InlineCacheState CollectTypeProfileNexus::StateFromFeedback() const {
   return MONOMORPHIC;
 }
 
-void CollectTypeProfileNexus::Collect(Handle<Name> type) {
+void CollectTypeProfileNexus::Collect(Handle<Name> type, int position) {
   Isolate* isolate = GetIsolate();
 
   Object* const feedback = GetFeedback();
@@ -1060,9 +1060,13 @@ void CollectTypeProfileNexus::Collect(Handle<Name> type) {
   } else {
     types = Handle<ArrayList>(ArrayList::cast(feedback), isolate);
   }
+
+  Handle<Tuple2> entry = isolate->factory()->NewTuple2(
+      type, Handle<Smi>(Smi::FromInt(position), isolate));
+
   // TODO(franzih): Somehow sort this list. Either avoid duplicates
   // or use the common base type.
-  SetFeedback(*ArrayList::Add(types, type));
+  SetFeedback(*ArrayList::Add(types, entry));
 }
 
 void CollectTypeProfileNexus::Print() const {
@@ -1078,8 +1082,13 @@ void CollectTypeProfileNexus::Print() const {
   list = Handle<ArrayList>(ArrayList::cast(feedback), isolate);
 
   for (int i = 0; i < list->Length(); i++) {
-    String* name = String::cast(list->Get(i));
-    PrintF("%s\n", name->ToCString().get());
+    Handle<Object> maybe_entry = Handle<Object>(list->Get(i), isolate);
+    DCHECK(maybe_entry->IsTuple2());
+    Handle<Tuple2> entry = Handle<Tuple2>::cast(maybe_entry);
+    Handle<String> type =
+        Handle<String>(String::cast(entry->value1()), isolate);
+    int position = Smi::cast(entry->value2())->value();
+    PrintF("%d: %s\n", position, type->ToCString().get());
   }
 }
 

src/feedback-vector.h

@@ -765,7 +765,7 @@ class CollectTypeProfileNexus : public FeedbackNexus {
   }
 
   // Add a type to the list of types.
-  void Collect(Handle<Name> type);
+  void Collect(Handle<Name> type, int position);
 
   // Dump the types to stdout.
   // TODO(franzih): pass this information to the debugger protocol instead of

src/interpreter/bytecode-array-builder.cc

@@ -640,9 +640,9 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::StoreDataPropertyInLiteral(
 }
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CollectTypeProfile(
-    Register name, int feedback_slot) {
+    int position, int feedback_slot) {
   DCHECK(FLAG_type_profile);
-  OutputCollectTypeProfile(name, feedback_slot);
+  OutputCollectTypeProfile(position, feedback_slot);
   return *this;
 }
 

src/interpreter/bytecode-array-builder.h

@@ -141,7 +141,7 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final
   // Collect type information for developer tools. The value for which we
   // record the type is stored in the accumulator.
   // TODO(franzih): Do not pass the name, instead use the source position.
-  BytecodeArrayBuilder& CollectTypeProfile(Register name, int feedback_slot);
+  BytecodeArrayBuilder& CollectTypeProfile(int position, int feedback_slot);
 
   // Store a property named by a property name. The value to be stored should be
   // in the accumulator.

src/interpreter/bytecode-generator.cc

@@ -1093,6 +1093,12 @@ void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
   builder()->SetStatementPosition(stmt);
   VisitForAccumulatorValue(stmt->expression());
 
+  if (stmt->HasTypeProfileSlot()) {
+    FeedbackSlot collect_type_feedback_slot = stmt->TypeProfileSlot();
+    builder()->CollectTypeProfile(stmt->position(),
+                                  feedback_index(collect_type_feedback_slot));
+  }
+
   if (stmt->is_async_return()) {
     execution_control()->AsyncReturnAccumulator();
   } else {
@@ -2211,34 +2217,18 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   LhsKind assign_type = Property::GetAssignType(property);
 
   // Evaluate LHS expression.
-  Register lhs_name;
-  if (expr->HasTypeProfileSlot()) {
-    lhs_name = register_allocator()->NewRegister();
-  }
-
   switch (assign_type) {
     case VARIABLE:
-      if (expr->HasTypeProfileSlot()) {
-        builder()
-            ->LoadLiteral(expr->target()->AsVariableProxy()->var()->raw_name())
-            .StoreAccumulatorInRegister(lhs_name);
-      }
       // Nothing to do to evaluate variable assignment LHS.
       break;
     case NAMED_PROPERTY: {
       object = VisitForRegisterValue(property->obj());
       name = property->key()->AsLiteral()->AsRawPropertyName();
-      if (expr->HasTypeProfileSlot()) {
-        builder()->LoadLiteral(name).StoreAccumulatorInRegister(lhs_name);
-      }
       break;
     }
     case KEYED_PROPERTY: {
       object = VisitForRegisterValue(property->obj());
       key = VisitForRegisterValue(property->key());
-      if (expr->HasTypeProfileSlot()) {
-        builder()->StoreAccumulatorInRegister(lhs_name);
-      }
       break;
     }
     case NAMED_SUPER_PROPERTY: {
@@ -2251,9 +2241,6 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
       builder()
           ->LoadLiteral(property->key()->AsLiteral()->AsRawPropertyName())
           .StoreAccumulatorInRegister(super_property_args[2]);
-      if (expr->HasTypeProfileSlot()) {
-        builder()->StoreAccumulatorInRegister(lhs_name);
-      }
       break;
     }
     case KEYED_SUPER_PROPERTY: {
@@ -2264,10 +2251,6 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
       VisitForRegisterValue(super_property->home_object(),
                             super_property_args[1]);
       VisitForRegisterValue(property->key(), super_property_args[2]);
-      if (expr->HasTypeProfileSlot()) {
-        builder()->StoreAccumulatorInRegister(lhs_name);
-      }
-
       break;
     }
   }
@@ -2358,10 +2341,12 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   }
 
   // Value is in accumulator.
-  if (expr->HasTypeProfileSlot()) {
+  // TODO(franzih): Collect type profile once we can handle more than just
+  // return statements.
+  if (false && expr->HasTypeProfileSlot()) {
     FeedbackSlot collect_type_feedback_slot = expr->TypeProfileSlot();
 
-    builder()->CollectTypeProfile(lhs_name,
+    builder()->CollectTypeProfile(expr->position(),
                                   feedback_index(collect_type_feedback_slot));
   }
 }

src/interpreter/bytecodes.h

@@ -105,7 +105,7 @@ namespace interpreter {
     OperandType::kReg, OperandType::kIdx)                                      \
   V(StaDataPropertyInLiteral, AccumulatorUse::kRead, OperandType::kReg,        \
     OperandType::kReg, OperandType::kFlag8, OperandType::kIdx)                 \
-  V(CollectTypeProfile, AccumulatorUse::kRead, OperandType::kReg,              \
+  V(CollectTypeProfile, AccumulatorUse::kRead, OperandType::kImm,              \
     OperandType::kIdx)                                                         \
                                                                                \
   /* Binary Operators */                                                       \

src/interpreter/interpreter-generator.cc

@@ -822,14 +822,14 @@ void InterpreterGenerator::DoStaDataPropertyInLiteral(
 
 void InterpreterGenerator::DoCollectTypeProfile(
     InterpreterAssembler* assembler) {
-  Node* name = __ LoadRegister(__ BytecodeOperandReg(0));
+  Node* position = __ BytecodeOperandImmSmi(0);
   Node* value = __ GetAccumulator();
   Node* vector_index = __ SmiTag(__ BytecodeOperandIdx(1));
 
   Node* feedback_vector = __ LoadFeedbackVector();
   Node* context = __ GetContext();
 
-  __ CallRuntime(Runtime::kCollectTypeProfile, context, name, value,
+  __ CallRuntime(Runtime::kCollectTypeProfile, context, position, value,
                  feedback_vector, vector_index);
   __ Dispatch();
 }

src/runtime/runtime-object.cc

@@ -694,7 +694,7 @@ RUNTIME_FUNCTION(Runtime_DefineDataPropertyInLiteral) {
 RUNTIME_FUNCTION(Runtime_CollectTypeProfile) {
   HandleScope scope(isolate);
   DCHECK_EQ(4, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Smi, position, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
   CONVERT_ARG_HANDLE_CHECKED(FeedbackVector, vector, 2);
   CONVERT_SMI_ARG_CHECKED(index, 3);
@@ -708,9 +708,9 @@ RUNTIME_FUNCTION(Runtime_CollectTypeProfile) {
   }
 
   CollectTypeProfileNexus nexus(vector, vector->ToSlot(index));
-  nexus.Collect(type);
+  nexus.Collect(type, position->value());
 
-  return *name;
+  return isolate->heap()->undefined_value();
 }
 
 // Return property without being observable by accessors or interceptors.

test/message/type-profile/collect-type-profile.js

@@ -4,25 +4,30 @@
 
 // Flags: --type-profile --turbo --allow-natives-syntax
 
-function test(param) {
-  var my_var1 = param;
-  var my_var2 = 17;
+function testFunction(param, flag) {
+  // We want to test 2 different return positions in one function.
+  if (flag) {
+    var first_var = param;
+    return first_var;
+  }
+  var second_var = param;
+  return second_var;
 }
 
-%PrintTypeProfile(test);
+%PrintTypeProfile(testFunction);
 
-test({});
-test(123);
-test('hello');
-test(123);
-%PrintTypeProfile(test);
+testFunction({});
+testFunction(123, true);
+testFunction('hello');
+testFunction(123);
+%PrintTypeProfile(testFunction);
 
-test(undefined);
-test('hello');
-test({x: 12});
-test({x: 12});
+testFunction(undefined);
+testFunction('hello', true);
+testFunction({x: 12}, true);
+testFunction({x: 12});
 
-%PrintTypeProfile(test);
+%PrintTypeProfile(testFunction);
 
 class MyClass {
   constructor() {}
@@ -37,4 +42,10 @@ testConstructorNames(new MyClass());
 testConstructorNames({});
 testConstructorNames(2);
 
+function testReturnOfNonVariable() {
+  return 32;
+}
+
+testReturnOfNonVariable();
+
 throw "throw otherwise test fails with --stress-opt";

test/message/type-profile/collect-type-profile.out

-Function: test
-Object
-number
-string
-number
+Function: testFunction
+424: Object
+374: number
+424: string
+424: number
 
-Function: test
-Object
-number
-string
-number
-undefined
-string
-Object
-Object
+Function: testFunction
+424: Object
+374: number
+424: string
+424: number
+424: undefined
+374: string
+374: Object
+424: Object
 
-*%(basename)s:40: throw otherwise test fails with --stress-opt
+*%(basename)s:51: throw otherwise test fails with --stress-opt
 throw "throw otherwise test fails with --stress-opt";
 ^