[assembler][x64] Add scoped CodeComment helper for nested comments

CodeComment nicely indents nested comments for better readable
disassembled code.

In addition, there are two helper macros:
- ASM_CODE_COMMENT adds the current function name as comment
- ASM_CODE_COMMENT_STRING macro can be used with custom strings

Bug: v8:11879
Change-Id: If5ff7e315f5acebe613f24b20d34694155f928d3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2960888
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75152}

src/baseline/baseline-compiler.cc

@@ -386,9 +386,8 @@ MemOperand BaselineCompiler::FeedbackVector() {
 }
 
 void BaselineCompiler::LoadFeedbackVector(Register output) {
-  __ RecordComment("[ LoadFeedbackVector");
+  ASM_CODE_COMMENT(&masm_);
   __ Move(output, __ FeedbackVectorOperand());
-  __ RecordComment("]");
 }
 
 void BaselineCompiler::LoadClosureFeedbackArray(Register output) {
@@ -463,12 +462,13 @@ void BaselineCompiler::VisitSingleBytecode() {
   // and exception handling, when CFI is enabled.
   __ JumpTarget();
 
+#ifdef V8_CODE_COMMENTS
+  std::ostringstream str;
   if (FLAG_code_comments) {
-    std::ostringstream str;
-    str << "[ ";
     iterator().PrintTo(str);
-    __ RecordComment(str.str().c_str());
   }
+  ASM_CODE_COMMENT_STRING(&masm_, str.str());
+#endif
 
   VerifyFrame();
 
@@ -484,7 +484,6 @@ void BaselineCompiler::VisitSingleBytecode() {
     BYTECODE_LIST(BYTECODE_CASE)
 #undef BYTECODE_CASE
   }
-  __ RecordComment("]");
 
 #ifdef V8_TRACE_UNOPTIMIZED
   TraceBytecode(Runtime::kTraceUnoptimizedBytecodeExit);
@@ -493,7 +492,7 @@ void BaselineCompiler::VisitSingleBytecode() {
 
 void BaselineCompiler::VerifyFrame() {
   if (FLAG_debug_code) {
-    __ RecordComment("[ Verify frame");
+    ASM_CODE_COMMENT(&masm_);
     __ RecordComment(" -- Verify frame size");
     VerifyFrameSize();
 
@@ -512,8 +511,6 @@ void BaselineCompiler::VerifyFrame() {
     }
 
     // TODO(leszeks): More verification.
-
-    __ RecordComment("]");
   }
 }
 
@@ -545,7 +542,7 @@ INTRINSICS_LIST(DECLARE_VISITOR)
 void BaselineCompiler::UpdateInterruptBudgetAndJumpToLabel(
     int weight, Label* label, Label* skip_interrupt_label) {
   if (weight != 0) {
-    __ RecordComment("[ Update Interrupt Budget");
+    ASM_CODE_COMMENT(&masm_);
     __ AddToInterruptBudgetAndJumpIfNotExceeded(weight, skip_interrupt_label);
 
     if (weight < 0) {
@@ -555,7 +552,6 @@ void BaselineCompiler::UpdateInterruptBudgetAndJumpToLabel(
     }
   }
   if (label) __ Jump(label);
-  if (weight != 0) __ RecordComment("]");
 }
 
 void BaselineCompiler::UpdateInterruptBudgetAndDoInterpreterJump() {
@@ -591,10 +587,9 @@ Label* BaselineCompiler::BuildForwardJumpLabel() {
 
 template <Builtin kBuiltin, typename... Args>
 void BaselineCompiler::CallBuiltin(Args... args) {
-  __ RecordComment("[ CallBuiltin");
+  ASM_CODE_COMMENT(&masm_);
   detail::MoveArgumentsForBuiltin<kBuiltin>(&basm_, args...);
   __ CallBuiltin(kBuiltin);
-  __ RecordComment("]");
 }
 
 template <Builtin kBuiltin, typename... Args>
@@ -1940,15 +1935,17 @@ void BaselineCompiler::VisitJumpLoop() {
   BaselineAssembler::ScratchRegisterScope scope(&basm_);
   Register scratch = scope.AcquireScratch();
   Label osr_not_armed;
-  __ RecordComment("[ OSR Check Armed");
-  Register osr_level = scratch;
-  __ LoadRegister(osr_level, interpreter::Register::bytecode_array());
-  __ LoadByteField(osr_level, osr_level, BytecodeArray::kOsrNestingLevelOffset);
-  int loop_depth = iterator().GetImmediateOperand(1);
-  __ JumpIfByte(Condition::kUnsignedLessThanEqual, osr_level, loop_depth,
-                &osr_not_armed);
-  CallBuiltin<Builtin::kBaselineOnStackReplacement>();
-  __ RecordComment("]");
+  {
+    ASM_CODE_COMMENT_STRING(&masm_, "OSR Check Armed");
+    Register osr_level = scratch;
+    __ LoadRegister(osr_level, interpreter::Register::bytecode_array());
+    __ LoadByteField(osr_level, osr_level,
+                     BytecodeArray::kOsrNestingLevelOffset);
+    int loop_depth = iterator().GetImmediateOperand(1);
+    __ JumpIfByte(Condition::kUnsignedLessThanEqual, osr_level, loop_depth,
+                  &osr_not_armed);
+    CallBuiltin<Builtin::kBaselineOnStackReplacement>();
+  }
 
   __ Bind(&osr_not_armed);
   Label* label = &labels_[iterator().GetJumpTargetOffset()]->unlinked;
@@ -2147,7 +2144,7 @@ void BaselineCompiler::VisitReThrow() {
 }
 
 void BaselineCompiler::VisitReturn() {
-  __ RecordComment("[ Return");
+  ASM_CODE_COMMENT_STRING(&masm_, "Return");
   int profiling_weight = iterator().current_offset() +
                          iterator().current_bytecode_size_without_prefix();
   int parameter_count = bytecode_->parameter_count();
@@ -2159,7 +2156,6 @@ void BaselineCompiler::VisitReturn() {
                             // computation. We'll account for it at the end.
   TailCallBuiltin<Builtin::kBaselineLeaveFrame>(
       parameter_count_without_receiver, -profiling_weight);
-  __ RecordComment("]");
 }
 
 void BaselineCompiler::VisitThrowReferenceErrorIfHole() {

src/baseline/x64/baseline-assembler-x64-inl.h

@@ -122,9 +122,9 @@ void BaselineAssembler::CallBuiltin(Builtin builtin) {
     // Generate pc-relative call.
     __ CallBuiltin(builtin);
   } else {
-    __ RecordCommentForOffHeapTrampoline(builtin);
+    ASM_CODE_COMMENT_STRING(masm_,
+                            __ CommentForOffHeapTrampoline("call", builtin));
     __ Call(__ EntryFromBuiltinAsOperand(builtin));
-    __ RecordComment("]");
   }
 }
 
@@ -133,9 +133,9 @@ void BaselineAssembler::TailCallBuiltin(Builtin builtin) {
     // Generate pc-relative jump.
     __ TailCallBuiltin(builtin);
   } else {
-    __ RecordCommentForOffHeapTrampoline(builtin);
+    ASM_CODE_COMMENT_STRING(
+        masm_, __ CommentForOffHeapTrampoline("tail call", builtin));
     __ Jump(__ EntryFromBuiltinAsOperand(builtin));
-    __ RecordComment("]");
   }
 }
 

src/baseline/x64/baseline-compiler-x64-inl.h

@@ -16,6 +16,7 @@ namespace baseline {
 #define __ basm_.
 
 void BaselineCompiler::Prologue() {
+  ASM_CODE_COMMENT(&masm_);
   DCHECK_EQ(kJSFunctionRegister, kJavaScriptCallTargetRegister);
   int max_frame_size = bytecode_->frame_size() + max_call_args_;
   CallBuiltin<Builtin::kBaselineOutOfLinePrologue>(
@@ -26,7 +27,7 @@ void BaselineCompiler::Prologue() {
 }
 
 void BaselineCompiler::PrologueFillFrame() {
-  __ RecordComment("[ Fill frame");
+  ASM_CODE_COMMENT(&masm_);
   // Inlined register frame fill
   interpreter::Register new_target_or_generator_register =
       bytecode_->incoming_new_target_or_generator_register();
@@ -74,10 +75,10 @@ void BaselineCompiler::PrologueFillFrame() {
     __ masm()->decl(scratch);
     __ masm()->j(greater, &loop);
   }
-  __ RecordComment("]");
 }
 
 void BaselineCompiler::VerifyFrameSize() {
+  ASM_CODE_COMMENT(&masm_);
   __ Move(kScratchRegister, rsp);
   __ masm()->addq(kScratchRegister,
                   Immediate(InterpreterFrameConstants::kFixedFrameSizeFromFp +

src/codegen/assembler.cc

@@ -34,6 +34,9 @@
 
 #include "src/codegen/assembler.h"
 
+#ifdef V8_CODE_COMMENTS
+#include <iomanip>
+#endif
 #include "src/codegen/assembler-inl.h"
 #include "src/codegen/string-constants.h"
 #include "src/deoptimizer/deoptimizer.h"
@@ -308,5 +311,24 @@ int Assembler::WriteCodeComments() {
   return size;
 }
 
+#ifdef V8_CODE_COMMENTS
+int Assembler::CodeComment::depth() const { return assembler_->comment_depth_; }
+void Assembler::CodeComment::Open(const std::string& comment) {
+  std::stringstream sstream;
+  sstream << std::setfill(' ') << std::setw(depth() * kIndentWidth + 2);
+  sstream << "[ " << comment;
+  assembler_->comment_depth_++;
+  assembler_->RecordComment(sstream.str());
+}
+
+void Assembler::CodeComment::Close() {
+  assembler_->comment_depth_--;
+  std::string comment = "]";
+  comment.insert(0, depth() * kIndentWidth, ' ');
+  DCHECK_LE(0, depth());
+  assembler_->RecordComment(comment);
+}
+#endif
+
 }  // namespace internal
 }  // namespace v8

src/codegen/assembler.h

@@ -288,15 +288,48 @@ class V8_EXPORT_PRIVATE AssemblerBase : public Malloced {
 
   // Record an inline code comment that can be used by a disassembler.
   // Use --code-comments to enable.
-  V8_INLINE void RecordComment(const char* msg) {
+  V8_INLINE void RecordComment(const char* comment) {
     // Set explicit dependency on --code-comments for dead-code elimination in
     // release builds.
     if (!FLAG_code_comments) return;
     if (options().emit_code_comments) {
-      code_comments_writer_.Add(pc_offset(), std::string(msg));
+      code_comments_writer_.Add(pc_offset(), std::string(comment));
     }
   }
 
+  V8_INLINE void RecordComment(std::string comment) {
+    // Set explicit dependency on --code-comments for dead-code elimination in
+    // release builds.
+    if (!FLAG_code_comments) return;
+    if (options().emit_code_comments) {
+      code_comments_writer_.Add(pc_offset(), std::move(comment));
+    }
+  }
+
+#ifdef V8_CODE_COMMENTS
+  class CodeComment {
+   public:
+    explicit CodeComment(Assembler* assembler, const std::string& comment)
+        : assembler_(assembler) {
+      if (FLAG_code_comments) Open(comment);
+    }
+    ~CodeComment() {
+      if (FLAG_code_comments) Close();
+    }
+    static const int kIndentWidth = 2;
+
+   private:
+    int depth() const;
+    void Open(const std::string& comment);
+    void Close();
+    Assembler* assembler_;
+  };
+#else  // V8_CODE_COMMENTS
+  class CodeComment {
+    explicit CodeComment(Assembler* assembler, std::string comment) {}
+  };
+#endif
+
   // The minimum buffer size. Should be at least two times the platform-specific
   // {Assembler::kGap}.
   static constexpr int kMinimalBufferSize = 128;
@@ -386,6 +419,10 @@ class V8_EXPORT_PRIVATE AssemblerBase : public Malloced {
 
   JumpOptimizationInfo* jump_optimization_info_;
 
+#ifdef V8_CODE_COMMENTS
+  int comment_depth_ = 0;
+#endif
+
   // Constant pool.
   friend class FrameAndConstantPoolScope;
   friend class ConstantPoolUnavailableScope;
@@ -416,6 +453,15 @@ class V8_EXPORT_PRIVATE V8_NODISCARD CpuFeatureScope {
 #endif
 };
 
+#ifdef V8_CODE_COMMENTS
+#define ASM_CODE_COMMENT(asm) ASM_CODE_COMMENT_STRING(asm, __func__)
+#define ASM_CODE_COMMENT_STRING(asm, comment) \
+  AssemblerBase::CodeComment asm_code_comment(asm, comment)
+#else
+#define ASM_CODE_COMMENT(asm)
+#define ASM_CODE_COMMENT_STRING(asm, ...)
+#endif
+
 }  // namespace internal
 }  // namespace v8
 #endif  // V8_CODEGEN_ASSEMBLER_H_

src/codegen/turbo-assembler.h

@@ -99,6 +99,15 @@ class V8_EXPORT_PRIVATE TurboAssemblerBase : public Assembler {
   static constexpr int kStackPageSize = 4 * KB;
 #endif
 
+  V8_INLINE std::string CommentForOffHeapTrampoline(const char* prefix,
+                                                    Builtin builtin) {
+    if (!FLAG_code_comments) return "";
+    std::ostringstream str;
+    str << "Inlined  Trampoline for " << prefix << " to "
+        << Builtins::name(builtin);
+    return str.str();
+  }
+
   V8_INLINE void RecordCommentForOffHeapTrampoline(Builtin builtin) {
     if (!FLAG_code_comments) return;
     std::ostringstream str;
@@ -128,6 +137,8 @@ class V8_EXPORT_PRIVATE TurboAssemblerBase : public Assembler {
 
   bool has_frame_ = false;
 
+  int comment_depth_ = 0;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(TurboAssemblerBase);
 };