[turbofan] Improve --trace-turbo-inlining

Instead of logging when SFI inlineability is computed, log when
that result is actually used.

Moreover, log something in some cases where we logged nothing
so far (e.g. when we can't inline due to missing feedback vector).

Change-Id: Id93119f4ead17aa9e721eb4fcc20774c54178665
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1873693Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64466}

src/compiler/heap-refs.h

@@ -761,21 +761,21 @@ class ScopeInfoRef : public HeapObjectRef {
   int ContextLength() const;
 };
 
-#define BROKER_SFI_FIELDS(V)                 \
-  V(int, internal_formal_parameter_count)    \
-  V(bool, has_duplicate_parameters)          \
-  V(int, function_map_index)                 \
-  V(FunctionKind, kind)                      \
-  V(LanguageMode, language_mode)             \
-  V(bool, native)                            \
-  V(bool, HasBreakInfo)                      \
-  V(bool, HasBuiltinId)                      \
-  V(bool, construct_as_builtin)              \
-  V(bool, HasBytecodeArray)                  \
-  V(bool, is_safe_to_skip_arguments_adaptor) \
-  V(bool, IsInlineable)                      \
-  V(int, StartPosition)                      \
-  V(bool, is_compiled)                       \
+#define BROKER_SFI_FIELDS(V)                             \
+  V(int, internal_formal_parameter_count)                \
+  V(bool, has_duplicate_parameters)                      \
+  V(int, function_map_index)                             \
+  V(FunctionKind, kind)                                  \
+  V(LanguageMode, language_mode)                         \
+  V(bool, native)                                        \
+  V(bool, HasBreakInfo)                                  \
+  V(bool, HasBuiltinId)                                  \
+  V(bool, construct_as_builtin)                          \
+  V(bool, HasBytecodeArray)                              \
+  V(bool, is_safe_to_skip_arguments_adaptor)             \
+  V(SharedFunctionInfo::Inlineability, GetInlineability) \
+  V(int, StartPosition)                                  \
+  V(bool, is_compiled)                                   \
   V(bool, IsUserJavaScript)
 
 class V8_EXPORT_PRIVATE SharedFunctionInfoRef : public HeapObjectRef {
@@ -791,6 +791,10 @@ class V8_EXPORT_PRIVATE SharedFunctionInfoRef : public HeapObjectRef {
   BROKER_SFI_FIELDS(DECL_ACCESSOR)
 #undef DECL_ACCESSOR
 
+  bool IsInlineable() const {
+    return GetInlineability() == SharedFunctionInfo::kIsInlineable;
+  }
+
   bool IsSerializedForCompilation(FeedbackVectorRef feedback) const;
   void SetSerializedForCompilation(FeedbackVectorRef feedback);
 

src/compiler/js-inlining-heuristic.cc

@@ -16,9 +16,9 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-#define TRACE(...)                                      \
-  do {                                                  \
-    if (FLAG_trace_turbo_inlining) PrintF(__VA_ARGS__); \
+#define TRACE(...)                                                             \
+  do {                                                                         \
+    if (FLAG_trace_turbo_inlining) StdoutStream{} << __VA_ARGS__ << std::endl; \
   } while (false)
 
 namespace {
@@ -29,22 +29,38 @@ bool IsSmall(BytecodeArrayRef const& bytecode) {
 bool CanConsiderForInlining(JSHeapBroker* broker,
                             SharedFunctionInfoRef const& shared,
                             FeedbackVectorRef const& feedback_vector) {
-  if (!shared.IsInlineable()) return false;
+  SharedFunctionInfo::Inlineability inlineability = shared.GetInlineability();
+  if (inlineability != SharedFunctionInfo::kIsInlineable) {
+    TRACE("Cannot consider "
+          << shared << " for inlining (reason: " << inlineability << ")");
+    return false;
+  }
+
   DCHECK(shared.HasBytecodeArray());
   if (!shared.IsSerializedForCompilation(feedback_vector)) {
     TRACE_BROKER_MISSING(
         broker, "data for " << shared << " (not serialized for compilation)");
+    TRACE("Cannot consider " << shared << " for inlining with "
+                             << feedback_vector << " (missing data)");
     return false;
   }
+
+  TRACE("Considering " << shared << " for inlining with " << feedback_vector);
   return true;
 }
 
 bool CanConsiderForInlining(JSHeapBroker* broker,
                             JSFunctionRef const& function) {
-  if (!function.has_feedback_vector()) return false;
+  if (!function.has_feedback_vector()) {
+    TRACE("Cannot consider " << function
+                             << " for inlining (no feedback vector)");
+    return false;
+  }
+
   if (!function.serialized()) {
     TRACE_BROKER_MISSING(
         broker, "data for " << function << " (cannot consider for inlining)");
+    TRACE("Cannot consider " << function << " for inlining (missing data)");
     return false;
   }
   return CanConsiderForInlining(broker, function.shared(),
@@ -129,10 +145,9 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
   if (candidate.num_functions == 0) {
     return NoChange();
   } else if (candidate.num_functions > 1 && !FLAG_polymorphic_inlining) {
-    TRACE(
-        "Not considering call site #%d:%s, because polymorphic inlining "
-        "is disabled\n",
-        node->id(), node->op()->mnemonic());
+    TRACE("Not considering call site #"
+          << node->id() << ":" << node->op()->mnemonic()
+          << ", because polymorphic inlining is disabled");
     return NoChange();
   }
 
@@ -163,8 +178,9 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
     // not obvious if such an anlysis is needed.
     if (frame_info.shared_info().ToHandle(&frame_shared_info) &&
         frame_shared_info.equals(shared.object())) {
-      TRACE("Not considering call site #%d:%s, because of recursive inlining\n",
-            node->id(), node->op()->mnemonic());
+      TRACE("Not considering call site #" << node->id() << ":"
+                                          << node->op()->mnemonic()
+                                          << ", because of recursive inlining");
       candidate.can_inline_function[i] = false;
     }
     if (candidate.can_inline_function[i]) {
@@ -208,8 +224,8 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
   // Forcibly inline small functions here. In the case of polymorphic inlining
   // candidate_is_small is set only when all functions are small.
   if (candidate_is_small) {
-    TRACE("Inlining small function(s) at call site #%d:%s\n", node->id(),
-          node->op()->mnemonic());
+    TRACE("Inlining small function(s) at call site #"
+          << node->id() << ":" << node->op()->mnemonic());
     return InlineCandidate(candidate, true);
   }
 

src/compiler/js-inlining.cc

@@ -414,10 +414,10 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
   NodeProperties::IsExceptionalCall(node, &exception_target);
 
   // JSInliningHeuristic has already filtered candidates without a BytecodeArray
-  // by calling SharedFunctionInfoRef::IsInlineable. For the ones passing the
-  // IsInlineable check, the broker holds a reference to the bytecode array,
-  // which prevents it from getting flushed.  Therefore, the following check
-  // should always hold true.
+  // based on SharedFunctionInfoRef::GetInlineability. For the inlineable ones
+  // (kIsInlineable), the broker holds a reference to the bytecode array, which
+  // prevents it from getting flushed.  Therefore, the following check should
+  // always hold true.
   CHECK(shared_info->is_compiled());
 
   if (!FLAG_concurrent_inlining && info_->is_source_positions_enabled()) {

src/compiler/serializer-for-background-compilation.cc

@@ -1852,12 +1852,13 @@ void SerializerForBackgroundCompilation::ProcessCalleeForCallOrConstruct(
   Handle<SharedFunctionInfo> shared = callee.shared(broker()->isolate());
   if (shared->IsApiFunction()) {
     ProcessApiCall(shared, arguments);
-    DCHECK(!shared->IsInlineable());
+    DCHECK_NE(shared->GetInlineability(), SharedFunctionInfo::kIsInlineable);
   } else if (shared->HasBuiltinId()) {
     ProcessBuiltinCall(shared, new_target, arguments, speculation_mode,
                        padding);
-    DCHECK(!shared->IsInlineable());
-  } else if (shared->IsInlineable() && callee.HasFeedbackVector()) {
+    DCHECK_NE(shared->GetInlineability(), SharedFunctionInfo::kIsInlineable);
+  } else if (shared->GetInlineability() == SharedFunctionInfo::kIsInlineable &&
+             callee.HasFeedbackVector()) {
     CompilationSubject subject =
         callee.ToCompilationSubject(broker()->isolate(), zone());
     environment()->accumulator_hints().Add(

src/objects/objects.cc

@@ -4640,7 +4640,7 @@ bool Script::GetPositionInfo(Handle<Script> script, int position,
   return script->GetPositionInfo(position, info, offset_flag);
 }
 
-bool Script::IsUserJavaScript() { return type() == Script::TYPE_NORMAL; }
+bool Script::IsUserJavaScript() const { return type() == Script::TYPE_NORMAL; }
 
 bool Script::ContainsAsmModule() {
   DisallowHeapAllocation no_gc;
@@ -5216,64 +5216,34 @@ Handle<Object> SharedFunctionInfo::GetSourceCodeHarmony(
   return builder.Finish().ToHandleChecked();
 }
 
-namespace {
-void TraceInlining(SharedFunctionInfo shared, const char* msg) {
-  if (FLAG_trace_turbo_inlining) {
-    StdoutStream os;
-    os << Brief(shared) << ": IsInlineable? " << msg << "\n";
-  }
-}
-}  // namespace
-
-bool SharedFunctionInfo::IsInlineable() {
-  if (!script().IsScript()) {
-    TraceInlining(*this, "false (no Script associated with it)");
-    return false;
-  }
+SharedFunctionInfo::Inlineability SharedFunctionInfo::GetInlineability() const {
+  if (!script().IsScript()) return kHasNoScript;
 
   if (GetIsolate()->is_precise_binary_code_coverage() &&
       !has_reported_binary_coverage()) {
     // We may miss invocations if this function is inlined.
-    TraceInlining(*this, "false (requires reported binary coverage)");
-    return false;
+    return kNeedsBinaryCoverage;
   }
 
-  if (optimization_disabled()) {
-    TraceInlining(*this, "false (optimization disabled)");
-    return false;
-  }
+  if (optimization_disabled()) return kHasOptimizationDisabled;
 
   // Built-in functions are handled by the JSCallReducer.
-  if (HasBuiltinId()) {
-    TraceInlining(*this, "false (is a builtin)");
-    return false;
-  }
+  if (HasBuiltinId()) return kIsBuiltin;
 
-  if (!IsUserJavaScript()) {
-    TraceInlining(*this, "false (is not user code)");
-    return false;
-  }
+  if (!IsUserJavaScript()) return kIsNotUserCode;
 
   // If there is no bytecode array, it is either not compiled or it is compiled
   // with WebAssembly for the asm.js pipeline. In either case we don't want to
   // inline.
-  if (!HasBytecodeArray()) {
-    TraceInlining(*this, "false (has no BytecodeArray)");
-    return false;
-  }
+  if (!HasBytecodeArray()) return kHasNoBytecode;
 
   if (GetBytecodeArray().length() > FLAG_max_inlined_bytecode_size) {
-    TraceInlining(*this, "false (length > FLAG_max_inlined_bytecode_size)");
-    return false;
+    return kExceedsBytecodeLimit;
   }
 
-  if (HasBreakInfo()) {
-    TraceInlining(*this, "false (may contain break points)");
-    return false;
-  }
+  if (HasBreakInfo()) return kMayContainBreakPoints;
 
-  TraceInlining(*this, "true");
-  return true;
+  return kIsInlineable;
 }
 
 int SharedFunctionInfo::SourceSize() { return EndPosition() - StartPosition(); }

src/objects/script.h

@@ -180,7 +180,7 @@ class Script : public Struct {
   V8_EXPORT_PRIVATE bool GetPositionInfo(int position, PositionInfo* info,
                                          OffsetFlag offset_flag) const;
 
-  bool IsUserJavaScript();
+  bool IsUserJavaScript() const;
 
   // Wrappers for GetPositionInfo
   static int GetColumnNumber(Handle<Script> script, int code_offset);

src/objects/shared-function-info-inl.h

@@ -706,14 +706,14 @@ String SharedFunctionInfo::inferred_name() {
   return GetReadOnlyRoots().empty_string();
 }
 
-bool SharedFunctionInfo::IsUserJavaScript() {
+bool SharedFunctionInfo::IsUserJavaScript() const {
   Object script_obj = script();
   if (script_obj.IsUndefined()) return false;
   Script script = Script::cast(script_obj);
   return script.IsUserJavaScript();
 }
 
-bool SharedFunctionInfo::IsSubjectToDebugging() {
+bool SharedFunctionInfo::IsSubjectToDebugging() const {
   return IsUserJavaScript() && !HasAsmWasmData();
 }
 

src/objects/shared-function-info.h

@@ -490,10 +490,10 @@ class SharedFunctionInfo : public HeapObject {
   // - internal break points
   // - coverage and type profile
   // - error stack trace
-  inline bool IsSubjectToDebugging();
+  inline bool IsSubjectToDebugging() const;
 
   // Whether this function is defined in user-provided JavaScript code.
-  inline bool IsUserJavaScript();
+  inline bool IsUserJavaScript() const;
 
   // True if one can flush compiled code from this function, in such a way that
   // it can later be re-compiled.
@@ -517,8 +517,19 @@ class SharedFunctionInfo : public HeapObject {
   // Hence it takes the mode as an argument.
   inline bool ShouldFlushBytecode(BytecodeFlushMode mode);
 
-  // Check whether or not this function is inlineable.
-  bool IsInlineable();
+  enum Inlineability {
+    kIsInlineable,
+    // Different reasons for not being inlineable:
+    kHasNoScript,
+    kNeedsBinaryCoverage,
+    kHasOptimizationDisabled,
+    kIsBuiltin,
+    kIsNotUserCode,
+    kHasNoBytecode,
+    kExceedsBytecodeLimit,
+    kMayContainBreakPoints,
+  };
+  Inlineability GetInlineability() const;
 
   // Source size of this function.
   int SourceSize();