[ignition] Change --trace-ignition to a runtime flag

Generate the code (extra runtime calls) for --trace-ignition support at
compile time, based on a #define (similar to TRACE_MAPS). Then check for
--trace-ignition at run-time when deciding whether to actually print
anything. This should make --trace-ignition less painful to use.

Note that --trace-igition is disabled by default, even on debug builds.
It has to be enabled with the gn arg "v8_enable_trace_ignition=true"

As a drive-by, TRACE_MAPS is renamed to V8_TRACE_MAPS, for consistency,
and SFI unique index (needed both by --trace-ignition and --trace-maps)
is cleaned up to be behind another #define.

Change-Id: I8dd0c62d0e6b7ee9c75541d45eb729dc03acbee9
Reviewed-on: https://chromium-review.googlesource.com/506203
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45346}

BUILD.gn

@@ -67,14 +67,14 @@ declare_args() {
   # Sets -dOBJECT_PRINT.
   v8_enable_object_print = ""
 
-  # Sets -dTRACE_MAPS.
+  # Sets -dV8_TRACE_MAPS.
   v8_enable_trace_maps = ""
 
   # Sets -dV8_ENABLE_CHECKS.
   v8_enable_v8_checks = ""
 
-  # Builds the snapshot with --trace-ignition
-  v8_trace_ignition = false
+  # Sets -dV8_TRACE_IGNITION.
+  v8_enable_trace_ignition = false
 
   # With post mortem support enabled, metadata is embedded into libv8 that
   # describes various parameters of the VM for use by debuggers. See
@@ -228,7 +228,10 @@ config("features") {
     defines += [ "VERIFY_PREDICTABLE" ]
   }
   if (v8_enable_trace_maps) {
-    defines += [ "TRACE_MAPS" ]
+    defines += [ "V8_TRACE_MAPS" ]
+  }
+  if (v8_enable_trace_ignition) {
+    defines += [ "V8_TRACE_IGNITION" ]
   }
   if (v8_enable_v8_checks) {
     defines += [ "V8_ENABLE_CHECKS" ]
@@ -732,10 +735,6 @@ action("run_mksnapshot") {
     ]
   }
 
-  if (v8_trace_ignition) {
-    args += [ "--trace-ignition" ]
-  }
-
   if (v8_use_external_startup_data) {
     outputs += [ "$root_out_dir/snapshot_blob.bin" ]
     args += [

gypfiles/features.gypi

@@ -90,7 +90,7 @@
         'defines': ['VERIFY_HEAP',],
       }],
       ['v8_trace_maps==1', {
-        'defines': ['TRACE_MAPS',],
+        'defines': ['V8_TRACE_MAPS',],
       }],
       ['v8_enable_verify_predictable==1', {
         'defines': ['VERIFY_PREDICTABLE',],

gypfiles/toolchain.gypi

@@ -1245,7 +1245,7 @@
           'OBJECT_PRINT',
           'VERIFY_HEAP',
           'DEBUG',
-          'TRACE_MAPS'
+          'V8_TRACE_MAPS'
         ],
         'conditions': [
           ['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" or \

src/bootstrapper.cc

@@ -5117,7 +5117,7 @@ Genesis::Genesis(
     AddToWeakNativeContextList(*native_context());
     isolate->set_context(*native_context());
     isolate->counters()->contexts_created_by_snapshot()->Increment();
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
     if (FLAG_trace_maps) {
       Handle<JSFunction> object_fun = isolate->object_function();
       PrintF("[TraceMap: InitialMap map= %p SFI= %d_Object ]\n",

src/factory.cc

@@ -2489,7 +2489,7 @@ Handle<SharedFunctionInfo> Factory::NewSharedFunctionInfo(
       FeedbackMetadata::New(isolate(), &empty_spec);
   share->set_feedback_metadata(*feedback_metadata, SKIP_WRITE_BARRIER);
   share->set_function_literal_id(FunctionLiteral::kIdTypeInvalid);
-#if TRACE_MAPS
+#if V8_SFI_HAS_UNIQUE_ID
   share->set_unique_id(isolate()->GetNextUniqueSharedFunctionInfoId());
 #endif
   share->set_profiler_ticks(0);

src/flag-definitions.h

@@ -331,8 +331,10 @@ DEFINE_BOOL(print_bytecode, false,
             "print bytecode generated by ignition interpreter")
 DEFINE_STRING(print_bytecode_filter, "*",
               "filter for selecting which functions to print bytecode")
+#ifdef V8_TRACE_IGNITION
 DEFINE_BOOL(trace_ignition, false,
             "trace the bytecodes executed by the ignition interpreter")
+#endif
 DEFINE_BOOL(trace_ignition_codegen, false,
             "trace the codegen of ignition interpreter bytecode handlers")
 DEFINE_BOOL(trace_ignition_dispatches, false,
@@ -932,7 +934,7 @@ DEFINE_BOOL(trace_prototype_users, false,
             "Trace updates to prototype user tracking")
 DEFINE_BOOL(use_verbose_printer, true, "allows verbose printing")
 DEFINE_BOOL(trace_for_in_enumerate, false, "Trace for-in enumerate slow-paths")
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
 DEFINE_BOOL(trace_maps, false, "trace map creation")
 #endif
 

src/globals.h

@@ -120,6 +120,11 @@ namespace internal {
 
 #define V8_CONCURRENT_MARKING 0
 
+// Some types of tracing require the SFI to store a unique ID.
+#if defined(V8_TRACE_MAPS) || defined(V8_TRACE_IGNITION)
+#define V8_SFI_HAS_UNIQUE_ID 1
+#endif
+
 typedef uint8_t byte;
 typedef byte* Address;
 

src/interpreter/interpreter-assembler.cc

@@ -49,9 +49,9 @@ InterpreterAssembler::InterpreterAssembler(CodeAssemblerState* state,
   dispatch_table_.Bind(
       Parameter(InterpreterDispatchDescriptor::kDispatchTable));
 
-  if (FLAG_trace_ignition) {
-    TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry);
-  }
+#ifdef V8_TRACE_IGNITION
+  TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry);
+#endif
   RegisterCallGenerationCallbacks([this] { CallPrologue(); },
                                   [this] { CallEpilogue(); });
 
@@ -1012,9 +1012,9 @@ Node* InterpreterAssembler::Advance(int delta) {
 }
 
 Node* InterpreterAssembler::Advance(Node* delta, bool backward) {
-  if (FLAG_trace_ignition) {
-    TraceBytecode(Runtime::kInterpreterTraceBytecodeExit);
-  }
+#ifdef V8_TRACE_IGNITION
+  TraceBytecode(Runtime::kInterpreterTraceBytecodeExit);
+#endif
   Node* next_offset = backward ? IntPtrSub(BytecodeOffset(), delta)
                                : IntPtrAdd(BytecodeOffset(), delta);
   bytecode_offset_.Bind(next_offset);
@@ -1088,9 +1088,9 @@ void InterpreterAssembler::InlineStar() {
   bytecode_ = Bytecode::kStar;
   accumulator_use_ = AccumulatorUse::kNone;
 
-  if (FLAG_trace_ignition) {
-    TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry);
-  }
+#ifdef V8_TRACE_IGNITION
+  TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry);
+#endif
   StoreRegister(GetAccumulator(), BytecodeOperandReg(0));
 
   DCHECK_EQ(accumulator_use_, Bytecodes::GetAccumulatorUse(bytecode_));

src/isolate.cc

@@ -2346,7 +2346,7 @@ Isolate::Isolate(bool enable_serializer)
       optimizing_compile_dispatcher_(NULL),
       stress_deopt_count_(0),
       next_optimization_id_(0),
-#if TRACE_MAPS
+#if V8_SFI_HAS_UNIQUE_ID
       next_unique_sfi_id_(0),
 #endif
       is_running_microtasks_(false),

src/isolate.h

@@ -1167,7 +1167,7 @@ class Isolate {
 
   std::string GetTurboCfgFileName();
 
-#if TRACE_MAPS
+#if V8_SFI_HAS_UNIQUE_ID
   int GetNextUniqueSharedFunctionInfoId() { return next_unique_sfi_id_++; }
 #endif
 
@@ -1541,7 +1541,7 @@ class Isolate {
 
   int next_optimization_id_;
 
-#if TRACE_MAPS
+#if V8_SFI_HAS_UNIQUE_ID
   int next_unique_sfi_id_;
 #endif
 

src/objects-inl.h

@@ -5921,7 +5921,7 @@ ACCESSORS(SharedFunctionInfo, construct_stub, Code, kConstructStubOffset)
 ACCESSORS(SharedFunctionInfo, feedback_metadata, FeedbackMetadata,
           kFeedbackMetadataOffset)
 SMI_ACCESSORS(SharedFunctionInfo, function_literal_id, kFunctionLiteralIdOffset)
-#if TRACE_MAPS
+#if V8_SFI_HAS_UNIQUE_ID
 SMI_ACCESSORS(SharedFunctionInfo, unique_id, kUniqueIdOffset)
 #endif
 ACCESSORS(SharedFunctionInfo, instance_class_name, Object,

src/objects-printer.cc

@@ -1523,9 +1523,7 @@ void LayoutDescriptor::Print(std::ostream& os) {  // NOLINT
 
 #endif  // OBJECT_PRINT
 
-
-#if TRACE_MAPS
-
+#if V8_TRACE_MAPS
 
 void Name::NameShortPrint() {
   if (this->IsString()) {
@@ -1556,9 +1554,7 @@ int Name::NameShortPrint(Vector<char> str) {
   }
 }
 
-
-#endif  // TRACE_MAPS
-
+#endif  // V8_TRACE_MAPS
 
 #if defined(DEBUG) || defined(OBJECT_PRINT)
 // This method is only meant to be called from gdb for debugging purposes.

src/objects.cc

@@ -5699,7 +5699,7 @@ void JSObject::MigrateSlowToFast(Handle<JSObject> object,
 
   NotifyMapChange(old_map, new_map, isolate);
 
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   if (FLAG_trace_maps) {
     PrintF("[TraceMaps: SlowToFast from= %p to= %p reason= %s ]\n",
            reinterpret_cast<void*>(*old_map), reinterpret_cast<void*>(*new_map),
@@ -8655,7 +8655,7 @@ Handle<Map> Map::Normalize(Handle<Map> fast_map, PropertyNormalizationMode mode,
       cache->Set(fast_map, new_map);
       isolate->counters()->maps_normalized()->Increment();
     }
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
     if (FLAG_trace_maps) {
       PrintF("[TraceMaps: Normalize from= %p to= %p reason= %s ]\n",
              reinterpret_cast<void*>(*fast_map),
@@ -8814,8 +8814,7 @@ Handle<Map> Map::ShareDescriptor(Handle<Map> map,
   return result;
 }
 
-
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
 
 // static
 void Map::TraceTransition(const char* what, Map* from, Map* to, Name* name) {
@@ -8840,8 +8839,7 @@ void Map::TraceAllTransitions(Map* map) {
   }
 }
 
-#endif  // TRACE_MAPS
-
+#endif  // V8_TRACE_MAPS
 
 void Map::ConnectTransition(Handle<Map> parent, Handle<Map> child,
                             Handle<Name> name, SimpleTransitionFlag flag) {
@@ -8856,12 +8854,12 @@ void Map::ConnectTransition(Handle<Map> parent, Handle<Map> child,
   }
   if (parent->is_prototype_map()) {
     DCHECK(child->is_prototype_map());
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
     Map::TraceTransition("NoTransition", *parent, *child, *name);
 #endif
   } else {
     TransitionArray::Insert(parent, name, child, flag);
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
     Map::TraceTransition("Transition", *parent, *child, *name);
 #endif
   }
@@ -8893,7 +8891,7 @@ Handle<Map> Map::CopyReplaceDescriptors(
   } else {
     result->InitializeDescriptors(*descriptors, *layout_descriptor);
   }
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   if (FLAG_trace_maps &&
       // Mirror conditions above that did not call ConnectTransition().
       (map->is_prototype_map() ||
@@ -9093,7 +9091,7 @@ Handle<Map> Map::CopyForTransition(Handle<Map> map, const char* reason) {
     new_map->InitializeDescriptors(*new_descriptors, *new_layout_descriptor);
   }
 
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   if (FLAG_trace_maps) {
     PrintF("[TraceMaps: CopyForTransition from= %p to= %p reason= %s ]\n",
            reinterpret_cast<void*>(*map), reinterpret_cast<void*>(*new_map),
@@ -9276,7 +9274,7 @@ Handle<Map> Map::TransitionToDataProperty(Handle<Map> map, Handle<Name> name,
   if (!maybe_map.ToHandle(&result)) {
     Isolate* isolate = name->GetIsolate();
     const char* reason = "TooManyFastProperties";
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
     std::unique_ptr<ScopedVector<char>> buffer;
     if (FLAG_trace_maps) {
       ScopedVector<char> name_buffer(100);
@@ -12651,7 +12649,7 @@ void JSFunction::SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
   }
   function->set_prototype_or_initial_map(*map);
   map->SetConstructor(*function);
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   if (FLAG_trace_maps) {
     PrintF("[TraceMaps: InitialMap map= %p SFI= %d_%s ]\n",
            reinterpret_cast<void*>(*map), function->shared()->unique_id(),

src/objects.h

@@ -5120,7 +5120,7 @@ class Map: public HeapObject {
   // Returns true if given field is unboxed double.
   inline bool IsUnboxedDoubleField(FieldIndex index);
 
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   static void TraceTransition(const char* what, Map* from, Map* to, Name* name);
   static void TraceAllTransitions(Map* map);
 #endif
@@ -5931,9 +5931,9 @@ class SharedFunctionInfo: public HeapObject {
   inline int function_literal_id() const;
   inline void set_function_literal_id(int value);
 
-#if TRACE_MAPS
-  // [unique_id] - For --trace-maps purposes, an identifier that's persistent
-  // even if the GC moves this SharedFunctionInfo.
+#if V8_SFI_HAS_UNIQUE_ID
+  // [unique_id] - For tracing purposes, an identifier that's persistent even if
+  // the GC moves this SharedFunctionInfo.
   inline int unique_id() const;
   inline void set_unique_id(int value);
 #endif
@@ -6271,7 +6271,7 @@ class SharedFunctionInfo: public HeapObject {
       kFunctionIdentifierOffset + kPointerSize;
   static const int kFunctionLiteralIdOffset =
       kFeedbackMetadataOffset + kPointerSize;
-#if TRACE_MAPS
+#if V8_SFI_HAS_UNIQUE_ID
   static const int kUniqueIdOffset = kFunctionLiteralIdOffset + kPointerSize;
   static const int kLastPointerFieldOffset = kUniqueIdOffset;
 #else
@@ -7842,7 +7842,7 @@ class Name: public HeapObject {
   DECLARE_CAST(Name)
 
   DECLARE_PRINTER(Name)
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   void NameShortPrint();
   int NameShortPrint(Vector<char> str);
 #endif
@@ -7961,7 +7961,7 @@ class Symbol: public Name {
  private:
   const char* PrivateSymbolToName() const;
 
-#if TRACE_MAPS
+#if V8_TRACE_MAPS
   friend class Name;  // For PrivateSymbolToName.
 #endif
 

src/runtime/runtime-interpreter.cc

@@ -34,6 +34,8 @@ RUNTIME_FUNCTION(Runtime_InterpreterNewClosure) {
       static_cast<PretenureFlag>(pretenured_flag));
 }
 
+#ifdef V8_TRACE_IGNITION
+
 namespace {
 
 void AdvanceToOffsetForTracing(
@@ -109,17 +111,22 @@ void PrintRegisters(std::ostream& os, bool is_input,
 }  // namespace
 
 RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeEntry) {
+  if (!FLAG_trace_ignition) {
+    return isolate->heap()->undefined_value();
+  }
+
   SealHandleScope shs(isolate);
   DCHECK_EQ(3, args.length());
   CONVERT_ARG_HANDLE_CHECKED(BytecodeArray, bytecode_array, 0);
   CONVERT_SMI_ARG_CHECKED(bytecode_offset, 1);
   CONVERT_ARG_HANDLE_CHECKED(Object, accumulator, 2);
-  OFStream os(stdout);
 
   int offset = bytecode_offset - BytecodeArray::kHeaderSize + kHeapObjectTag;
   interpreter::BytecodeArrayIterator bytecode_iterator(bytecode_array);
   AdvanceToOffsetForTracing(bytecode_iterator, offset);
   if (offset == bytecode_iterator.current_offset()) {
+    OFStream os(stdout);
+
     // Print bytecode.
     const uint8_t* base_address = bytecode_array->GetFirstBytecodeAddress();
     const uint8_t* bytecode_address = base_address + offset;
@@ -137,6 +144,10 @@ RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeEntry) {
 }
 
 RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeExit) {
+  if (!FLAG_trace_ignition) {
+    return isolate->heap()->undefined_value();
+  }
+
   SealHandleScope shs(isolate);
   DCHECK_EQ(3, args.length());
   CONVERT_ARG_HANDLE_CHECKED(BytecodeArray, bytecode_array, 0);
@@ -160,6 +171,8 @@ RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeExit) {
   return isolate->heap()->undefined_value();
 }
 
+#endif
+
 RUNTIME_FUNCTION(Runtime_InterpreterAdvanceBytecodeOffset) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(2, args.length());

src/runtime/runtime.h

@@ -216,10 +216,17 @@ namespace internal {
   F(ForInFilter, 2, 1)              \
   F(ForInHasProperty, 2, 1)
 
+#ifdef V8_TRACE_IGNITION
+#define FOR_EACH_INTRINSIC_INTERPRETER_TRACE(F) \
+  F(InterpreterTraceBytecodeEntry, 3, 1)        \
+  F(InterpreterTraceBytecodeExit, 3, 1)
+#else
+#define FOR_EACH_INTRINSIC_INTERPRETER_TRACE(F)
+#endif
+
 #define FOR_EACH_INTRINSIC_INTERPRETER(F) \
+  FOR_EACH_INTRINSIC_INTERPRETER_TRACE(F) \
   F(InterpreterNewClosure, 4, 1)          \
-  F(InterpreterTraceBytecodeEntry, 3, 1)  \
-  F(InterpreterTraceBytecodeExit, 3, 1)   \
   F(InterpreterAdvanceBytecodeOffset, 2, 1)
 
 #define FOR_EACH_INTRINSIC_FUNCTION(F)     \

src/setup-isolate-deserialize.cc

@@ -20,14 +20,6 @@ void SetupIsolateDelegate::SetupBuiltins(Isolate* isolate,
 
 void SetupIsolateDelegate::SetupInterpreter(
     interpreter::Interpreter* interpreter, bool create_heap_objects) {
-#ifdef V8_USE_SNAPSHOT
-  if (FLAG_trace_ignition || FLAG_trace_ignition_codegen ||
-      FLAG_trace_ignition_dispatches) {
-    OFStream os(stdout);
-    os << "Warning: --trace-ignition-* flags must be passed at mksnapshot "
-       << "time or used with nosnapshot builds." << std::endl;
-  }
-#endif
   DCHECK(interpreter->IsDispatchTableInitialized());
 }