[turbofan] Add handling of jumps to the serializer

Implemented branching and merging of Environments to facilitate handling of
conditional and unconditional jumps in the SerializerForBackgroundCompilation.
Added tests and printing helpers for the Environment. The internal structure
of the hints was changed to ZoneSet to support avoiding of duplicates.
Alternative implementation considerations were documented here:
https://docs.google.com/document/d/1vCQYhtFPqXafSMweSnGD8l0TKEIB6cPV5UGMHJtpy8k/edit?ts=5bf7d341#heading=h.jx4br0df5qzm

R=neis@chromium.org

Bug: v8:7790
Change-Id: Ib929c75ddb7f7fb290a5ca28d4422680a1514a4f
Reviewed-on: https://chromium-review.googlesource.com/c/1451847Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59534}

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

@@ -4,6 +4,8 @@
 
 #include "src/compiler/serializer-for-background-compilation.h"
 
+#include <sstream>
+
 #include "src/compiler/js-heap-broker.h"
 #include "src/handles-inl.h"
 #include "src/interpreter/bytecode-array-iterator.h"
@@ -29,24 +31,22 @@ CompilationSubject::CompilationSubject(Handle<JSFunction> closure,
 Hints::Hints(Zone* zone)
     : constants_(zone), maps_(zone), function_blueprints_(zone) {}
 
-const ZoneVector<Handle<Object>>& Hints::constants() const {
-  return constants_;
-}
+const ConstantsSet& Hints::constants() const { return constants_; }
 
-const ZoneVector<Handle<Map>>& Hints::maps() const { return maps_; }
+const MapsSet& Hints::maps() const { return maps_; }
 
-const ZoneVector<FunctionBlueprint>& Hints::function_blueprints() const {
+const BlueprintsSet& Hints::function_blueprints() const {
   return function_blueprints_;
 }
 
 void Hints::AddConstant(Handle<Object> constant) {
-  constants_.push_back(constant);
+  constants_.insert(constant);
 }
 
-void Hints::AddMap(Handle<Map> map) { maps_.push_back(map); }
+void Hints::AddMap(Handle<Map> map) { maps_.insert(map); }
 
 void Hints::AddFunctionBlueprint(FunctionBlueprint function_blueprint) {
-  function_blueprints_.push_back(function_blueprint);
+  function_blueprints_.insert(function_blueprint);
 }
 
 void Hints::Add(const Hints& other) {
@@ -59,6 +59,27 @@ bool Hints::IsEmpty() const {
   return constants().empty() && maps().empty() && function_blueprints().empty();
 }
 
+std::ostream& operator<<(std::ostream& out,
+                         const FunctionBlueprint& blueprint) {
+  out << Brief(*blueprint.shared) << std::endl;
+  out << Brief(*blueprint.feedback_vector) << std::endl;
+  return out;
+}
+
+std::ostream& operator<<(std::ostream& out, const Hints& hints) {
+  !hints.constants().empty() &&
+      out << "\t\tConstants (" << hints.constants().size() << "):" << std::endl;
+  for (auto x : hints.constants()) out << Brief(*x) << std::endl;
+  !hints.maps().empty() && out << "\t\tMaps (" << hints.maps().size()
+                               << "):" << std::endl;
+  for (auto x : hints.maps()) out << Brief(*x) << std::endl;
+  !hints.function_blueprints().empty() &&
+      out << "\t\tBlueprints (" << hints.function_blueprints().size()
+          << "):" << std::endl;
+  for (auto x : hints.function_blueprints()) out << x;
+  return out;
+}
+
 void Hints::Clear() {
   constants_.clear();
   maps_.clear();
@@ -68,10 +89,18 @@ void Hints::Clear() {
 
 class SerializerForBackgroundCompilation::Environment : public ZoneObject {
  public:
-  Environment(Zone* zone, Isolate* isolate, CompilationSubject function);
+  Environment(Zone* zone, CompilationSubject function);
   Environment(Zone* zone, Isolate* isolate, CompilationSubject function,
               base::Optional<Hints> new_target, const HintsVector& arguments);
 
+  // When control flow bytecodes are encountered, e.g. a conditional jump,
+  // the current environment needs to be stashed together with the target jump
+  // address. Later, when this target bytecode is handled, the stashed
+  // environment will be merged into the current one.
+  void Merge(Environment* other);
+
+  friend std::ostream& operator<<(std::ostream& out, const Environment& env);
+
   FunctionBlueprint function() const { return function_; }
 
   Hints& accumulator_hints() { return environment_hints_[accumulator_index()]; }
@@ -122,7 +151,7 @@ class SerializerForBackgroundCompilation::Environment : public ZoneObject {
 };
 
 SerializerForBackgroundCompilation::Environment::Environment(
-    Zone* zone, Isolate* isolate, CompilationSubject function)
+    Zone* zone, CompilationSubject function)
     : zone_(zone),
       function_(function.blueprint()),
       parameter_count_(function_.shared->GetBytecodeArray()->parameter_count()),
@@ -141,7 +170,7 @@ SerializerForBackgroundCompilation::Environment::Environment(
 SerializerForBackgroundCompilation::Environment::Environment(
     Zone* zone, Isolate* isolate, CompilationSubject function,
     base::Optional<Hints> new_target, const HintsVector& arguments)
-    : Environment(zone, isolate, function) {
+    : Environment(zone, function) {
   // Copy the hints for the actually passed arguments, at most up to
   // the parameter_count.
   size_t param_count = static_cast<size_t>(parameter_count());
@@ -167,6 +196,44 @@ SerializerForBackgroundCompilation::Environment::Environment(
   }
 }
 
+void SerializerForBackgroundCompilation::Environment::Merge(
+    Environment* other) {
+  // Presumably the source and the target would have the same layout
+  // so this is enforced here.
+  CHECK_EQ(parameter_count(), other->parameter_count());
+  CHECK_EQ(register_count(), other->register_count());
+  CHECK_EQ(environment_hints_size(), other->environment_hints_size());
+
+  for (size_t i = 0; i < environment_hints_.size(); ++i) {
+    environment_hints_[i].Add(other->environment_hints_[i]);
+  }
+  return_value_hints_.Add(other->return_value_hints_);
+}
+
+std::ostream& operator<<(
+    std::ostream& out,
+    const SerializerForBackgroundCompilation::Environment& env) {
+  std::ostringstream output_stream;
+  output_stream << "Function ";
+  env.function_.shared->Name()->Print(output_stream);
+  output_stream << "Parameter count: " << env.parameter_count() << std::endl;
+  output_stream << "Register count: " << env.register_count() << std::endl;
+
+  output_stream << "Hints (" << env.environment_hints_.size() << "):\n";
+  for (size_t i = 0; i < env.environment_hints_.size(); ++i) {
+    if (env.environment_hints_[i].IsEmpty()) continue;
+
+    output_stream << "\tSlot " << i << std::endl;
+    output_stream << env.environment_hints_[i];
+  }
+  output_stream << "Return value:\n";
+  output_stream << env.return_value_hints_
+                << "===========================================\n";
+
+  out << output_stream.str();
+  return out;
+}
+
 int SerializerForBackgroundCompilation::Environment::RegisterToLocalIndex(
     interpreter::Register reg) const {
   // TODO(mslekova): We also want to gather hints for the context.
@@ -183,8 +250,8 @@ SerializerForBackgroundCompilation::SerializerForBackgroundCompilation(
     JSHeapBroker* broker, Zone* zone, Handle<JSFunction> closure)
     : broker_(broker),
       zone_(zone),
-      environment_(new (zone) Environment(zone, broker_->isolate(),
-                                          {closure, broker_->isolate()})) {
+      environment_(new (zone) Environment(zone, {closure, broker_->isolate()})),
+      stashed_environments_(zone) {
   JSFunctionRef(broker, closure).Serialize();
 }
 
@@ -194,7 +261,8 @@ SerializerForBackgroundCompilation::SerializerForBackgroundCompilation(
     : broker_(broker),
       zone_(zone),
       environment_(new (zone) Environment(zone, broker_->isolate(), function,
-                                          new_target, arguments)) {
+                                          new_target, arguments)),
+      stashed_environments_(zone) {
   Handle<JSFunction> closure;
   if (function.closure().ToHandle(&closure)) {
     JSFunctionRef(broker, closure).Serialize();
@@ -221,6 +289,7 @@ void SerializerForBackgroundCompilation::TraverseBytecode() {
   BytecodeArrayIterator iterator(bytecode_array.object());
 
   for (; !iterator.done(); iterator.Advance()) {
+    MergeAfterJump(&iterator);
     switch (iterator.current_bytecode()) {
 #define DEFINE_BYTECODE_CASE(name)     \
   case interpreter::Bytecode::k##name: \
@@ -461,6 +530,13 @@ Hints SerializerForBackgroundCompilation::RunChildSerializer(
     return RunChildSerializer(function, new_target, padded, false);
   }
 
+  if (FLAG_trace_heap_broker) {
+    std::ostream& out = broker()->Trace();
+    out << "\nWill run child serializer with environment:\n"
+        << "===========================================\n"
+        << *environment();
+  }
+
   SerializerForBackgroundCompilation child_serializer(
       broker(), zone(), function, new_target, arguments);
   return child_serializer.Run();
@@ -543,6 +619,25 @@ void SerializerForBackgroundCompilation::ProcessCallVarArgs(
   ProcessCallOrConstruct(callee, base::nullopt, arguments, slot);
 }
 
+void SerializerForBackgroundCompilation::ProcessJump(
+    interpreter::BytecodeArrayIterator* iterator) {
+  int jump_target = iterator->GetJumpTargetOffset();
+  int current_offset = iterator->current_offset();
+  if (current_offset >= jump_target) return;
+
+  stashed_environments_[jump_target] = new (zone()) Environment(*environment());
+}
+
+void SerializerForBackgroundCompilation::MergeAfterJump(
+    interpreter::BytecodeArrayIterator* iterator) {
+  int current_offset = iterator->current_offset();
+  auto stash = stashed_environments_.find(current_offset);
+  if (stash != stashed_environments_.end()) {
+    environment()->Merge(stash->second);
+    stashed_environments_.erase(stash);
+  }
+}
+
 void SerializerForBackgroundCompilation::VisitReturn(
     BytecodeArrayIterator* iterator) {
   environment()->return_value_hints().Add(environment()->accumulator_hints());
@@ -588,13 +683,13 @@ void SerializerForBackgroundCompilation::VisitConstructWithSpread(
   ProcessCallOrConstruct(callee, new_target, arguments, slot, true);
 }
 
-#define DEFINE_SKIPPED_JUMP(name, ...)                  \
+#define DEFINE_CLEAR_ENVIRONMENT(name, ...)             \
   void SerializerForBackgroundCompilation::Visit##name( \
       BytecodeArrayIterator* iterator) {                \
     environment()->ClearEphemeralHints();               \
   }
-CLEAR_ENVIRONMENT_LIST(DEFINE_SKIPPED_JUMP)
-#undef DEFINE_SKIPPED_JUMP
+CLEAR_ENVIRONMENT_LIST(DEFINE_CLEAR_ENVIRONMENT)
+#undef DEFINE_CLEAR_ENVIRONMENT
 
 #define DEFINE_CLEAR_ACCUMULATOR(name, ...)             \
   void SerializerForBackgroundCompilation::Visit##name( \
@@ -604,6 +699,29 @@ CLEAR_ENVIRONMENT_LIST(DEFINE_SKIPPED_JUMP)
 CLEAR_ACCUMULATOR_LIST(DEFINE_CLEAR_ACCUMULATOR)
 #undef DEFINE_CLEAR_ACCUMULATOR
 
+#define DEFINE_CONDITIONAL_JUMP(name, ...)              \
+  void SerializerForBackgroundCompilation::Visit##name( \
+      BytecodeArrayIterator* iterator) {                \
+    ProcessJump(iterator);                              \
+  }
+CONDITIONAL_JUMPS_LIST(DEFINE_CONDITIONAL_JUMP)
+#undef DEFINE_CONDITIONAL_JUMP
+
+#define DEFINE_UNCONDITIONAL_JUMP(name, ...)            \
+  void SerializerForBackgroundCompilation::Visit##name( \
+      BytecodeArrayIterator* iterator) {                \
+    ProcessJump(iterator);                              \
+    environment()->ClearEphemeralHints();               \
+  }
+UNCONDITIONAL_JUMPS_LIST(DEFINE_UNCONDITIONAL_JUMP)
+#undef DEFINE_UNCONDITIONAL_JUMP
+
+#define DEFINE_IGNORE(name, ...)                        \
+  void SerializerForBackgroundCompilation::Visit##name( \
+      BytecodeArrayIterator* iterator) {}
+INGORED_BYTECODE_LIST(DEFINE_IGNORE)
+#undef DEFINE_IGNORE
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

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

@@ -36,27 +36,6 @@ namespace compiler {
   V(CreateFunctionContext)        \
   V(CreateEvalContext)            \
   V(Debugger)                     \
-  V(Jump)                         \
-  V(JumpConstant)                 \
-  V(JumpIfFalse)                  \
-  V(JumpIfFalseConstant)          \
-  V(JumpIfJSReceiver)             \
-  V(JumpIfJSReceiverConstant)     \
-  V(JumpIfNotNull)                \
-  V(JumpIfNotNullConstant)        \
-  V(JumpIfNotUndefined)           \
-  V(JumpIfNotUndefinedConstant)   \
-  V(JumpIfNull)                   \
-  V(JumpIfNullConstant)           \
-  V(JumpIfToBooleanTrueConstant)  \
-  V(JumpIfToBooleanFalseConstant) \
-  V(JumpIfToBooleanTrue)          \
-  V(JumpIfToBooleanFalse)         \
-  V(JumpIfTrue)                   \
-  V(JumpIfTrueConstant)           \
-  V(JumpIfUndefined)              \
-  V(JumpIfUndefinedConstant)      \
-  V(JumpLoop)                     \
   V(PushContext)                  \
   V(PopContext)                   \
   V(ResumeGenerator)              \
@@ -65,10 +44,7 @@ namespace compiler {
   V(StaCurrentContextSlot)        \
   V(SuspendGenerator)             \
   V(SwitchOnGeneratorState)       \
-  V(Throw)                        \
-  V(ThrowReferenceErrorIfHole)    \
-  V(ThrowSuperNotCalledIfHole)    \
-  V(ThrowSuperAlreadyCalledIfNotHole)
+  V(Throw)
 
 #define CLEAR_ACCUMULATOR_LIST(V)   \
   V(CreateEmptyObjectLiteral)       \
@@ -85,6 +61,49 @@ namespace compiler {
   V(LdaNamedProperty)               \
   V(LdaNamedPropertyNoFeedback)
 
+#define UNCONDITIONAL_JUMPS_LIST(V) \
+  V(Jump)                           \
+  V(JumpConstant)                   \
+  V(JumpLoop)
+
+#define CONDITIONAL_JUMPS_LIST(V) \
+  V(JumpIfFalse)                  \
+  V(JumpIfFalseConstant)          \
+  V(JumpIfJSReceiver)             \
+  V(JumpIfJSReceiverConstant)     \
+  V(JumpIfNotNull)                \
+  V(JumpIfNotNullConstant)        \
+  V(JumpIfNotUndefined)           \
+  V(JumpIfNotUndefinedConstant)   \
+  V(JumpIfNull)                   \
+  V(JumpIfNullConstant)           \
+  V(JumpIfToBooleanTrueConstant)  \
+  V(JumpIfToBooleanFalseConstant) \
+  V(JumpIfToBooleanTrue)          \
+  V(JumpIfToBooleanFalse)         \
+  V(JumpIfTrue)                   \
+  V(JumpIfTrueConstant)           \
+  V(JumpIfUndefined)              \
+  V(JumpIfUndefinedConstant)
+
+#define INGORED_BYTECODE_LIST(V) \
+  V(TestEqual)                   \
+  V(TestEqualStrict)             \
+  V(TestLessThan)                \
+  V(TestGreaterThan)             \
+  V(TestLessThanOrEqual)         \
+  V(TestGreaterThanOrEqual)      \
+  V(TestReferenceEqual)          \
+  V(TestInstanceOf)              \
+  V(TestIn)                      \
+  V(TestUndetectable)            \
+  V(TestNull)                    \
+  V(TestUndefined)               \
+  V(TestTypeOf)                  \
+  V(ThrowReferenceErrorIfHole)   \
+  V(ThrowSuperNotCalledIfHole)   \
+  V(ThrowSuperAlreadyCalledIfNotHole)
+
 #define SUPPORTED_BYTECODE_LIST(V) \
   V(CallAnyReceiver)               \
   V(CallNoFeedback)                \
@@ -114,13 +133,30 @@ namespace compiler {
   V(Star)                          \
   V(Wide)                          \
   CLEAR_ENVIRONMENT_LIST(V)        \
-  CLEAR_ACCUMULATOR_LIST(V)
+  CLEAR_ACCUMULATOR_LIST(V)        \
+  CONDITIONAL_JUMPS_LIST(V)        \
+  UNCONDITIONAL_JUMPS_LIST(V)      \
+  INGORED_BYTECODE_LIST(V)
 
 class JSHeapBroker;
 
+template <typename T>
+struct HandleComparator {
+  bool operator()(const Handle<T>& lhs, const Handle<T>& rhs) const {
+    return lhs.address() < rhs.address();
+  }
+};
+
 struct FunctionBlueprint {
   Handle<SharedFunctionInfo> shared;
   Handle<FeedbackVector> feedback_vector;
+
+  bool operator<(const FunctionBlueprint& other) const {
+    // A feedback vector is never used for more than one SFI, so it could
+    // be used for strict ordering of blueprints.
+    return HandleComparator<FeedbackVector>()(feedback_vector,
+                                              other.feedback_vector);
+  }
 };
 
 class CompilationSubject {
@@ -137,13 +173,17 @@ class CompilationSubject {
   MaybeHandle<JSFunction> closure_;
 };
 
+typedef ZoneSet<Handle<Object>, HandleComparator<Object>> ConstantsSet;
+typedef ZoneSet<Handle<Map>, HandleComparator<Map>> MapsSet;
+typedef ZoneSet<FunctionBlueprint> BlueprintsSet;
+
 class Hints {
  public:
   explicit Hints(Zone* zone);
 
-  const ZoneVector<Handle<Object>>& constants() const;
-  const ZoneVector<Handle<Map>>& maps() const;
-  const ZoneVector<FunctionBlueprint>& function_blueprints() const;
+  const ConstantsSet& constants() const;
+  const MapsSet& maps() const;
+  const BlueprintsSet& function_blueprints() const;
 
   void AddConstant(Handle<Object> constant);
   void AddMap(Handle<Map> map);
@@ -155,9 +195,9 @@ class Hints {
   bool IsEmpty() const;
 
  private:
-  ZoneVector<Handle<Object>> constants_;
-  ZoneVector<Handle<Map>> maps_;
-  ZoneVector<FunctionBlueprint> function_blueprints_;
+  ConstantsSet constants_;
+  MapsSet maps_;
+  BlueprintsSet function_blueprints_;
 };
 
 typedef ZoneVector<Hints> HintsVector;
@@ -171,6 +211,8 @@ class SerializerForBackgroundCompilation {
                                      Handle<JSFunction> closure);
   Hints Run();  // NOTE: Returns empty for an already-serialized function.
 
+  class Environment;
+
  private:
   SerializerForBackgroundCompilation(JSHeapBroker* broker, Zone* zone,
                                      CompilationSubject function,
@@ -184,14 +226,14 @@ class SerializerForBackgroundCompilation {
   SUPPORTED_BYTECODE_LIST(DECLARE_VISIT_BYTECODE)
 #undef DECLARE_VISIT_BYTECODE
 
-  class Environment;
-
   void ProcessCallOrConstruct(Hints callee, base::Optional<Hints> new_target,
                               const HintsVector& arguments, FeedbackSlot slot,
                               bool with_spread = false);
   void ProcessCallVarArgs(interpreter::BytecodeArrayIterator* iterator,
                           ConvertReceiverMode receiver_mode,
                           bool with_spread = false);
+  void ProcessJump(interpreter::BytecodeArrayIterator* iterator);
+  void MergeAfterJump(interpreter::BytecodeArrayIterator* iterator);
 
   Hints RunChildSerializer(CompilationSubject function,
                            base::Optional<Hints> new_target,
@@ -204,6 +246,7 @@ class SerializerForBackgroundCompilation {
   JSHeapBroker* const broker_;
   Zone* const zone_;
   Environment* const environment_;
+  ZoneUnorderedMap<int, Environment*> stashed_environments_;
 };
 
 }  // namespace compiler

test/cctest/compiler/serializer-tester.cc

@@ -28,6 +28,8 @@ SerializerTester::SerializerTester(const char* source)
   FLAG_use_ic = true;
   // We need manual control over when a given function is optimized.
   FLAG_always_opt = false;
+  // We need allocation of executable memory for the compilation.
+  FLAG_jitless = false;
 
   std::string function_string = "(function() { ";
   function_string += source;
@@ -180,6 +182,36 @@ TEST(SerializeConstructWithSpread) {
       "}; f(); return f;");
 }
 
+TEST(SerializeConditionalJump) {
+  CheckForSerializedInlinee(
+      "function g(callee) { callee(); };"
+      "function h() {};"
+      "function i() {};"
+      "let a = true;"
+      "g(h); g(i);"
+      "function f() {"
+      "  function q() {};"
+      "  if (a) g(q);"
+      "  return q;"
+      "}; f(); return f;");
+}
+
+TEST(SerializeUnconditionalJump) {
+  CheckForSerializedInlinee(
+      "function g(callee) { callee(); };"
+      "function h() {};"
+      "function i() {};"
+      "let a = false;"
+      "g(h); g(i);"
+      "function f() {"
+      "  function p() {};"
+      "  function q() {};"
+      "  if (a) g(q);"
+      "  else g(p);"
+      "  return p;"
+      "}; f(); return f;");
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8