Various improvements regarding the way we print code code comments.

src/ia32/lithium-codegen-ia32.cc

Index: src/ia32/lithium-codegen-ia32.cc
diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 221882ca043e1c9d6f3b7d6f205287fac22995c5..39934fc76e5b013b992b684d4ec78d463e9a9efb 100644
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -336,49 +336,28 @@ bool LCodeGen::GenerateBody() {
        !is_aborted() && current_instruction_ < instructions_->length();
        current_instruction_++) {
     LInstruction* instr = instructions_->at(current_instruction_);
+
+    // Don't emit code for basic blocks with a replacement.
     if (instr->IsLabel()) {
-      LLabel* label = LLabel::cast(instr);
-      emit_instructions = !label->HasReplacement();
+      emit_instructions = !LLabel::cast(instr)->HasReplacement();
     }
+    if (!emit_instructions) continue;
 
-    if (emit_instructions) {
-      if (FLAG_code_comments) {
-        HValue* hydrogen = instr->hydrogen_value();
-        if (hydrogen != NULL) {
-          if (hydrogen->IsChange()) {
-            HValue* changed_value = HChange::cast(hydrogen)->value();
-            int use_id = 0;
-            const char* use_mnemo = "dead";
-            if (hydrogen->UseCount() >= 1) {
-              HValue* use_value = hydrogen->uses().value();
-              use_id = use_value->id();
-              use_mnemo = use_value->Mnemonic();
-            }
-            Comment(";;; @%d: %s. <of #%d %s for #%d %s>",

[Jakob Kummerow, 2013/04/22 09:19:59]

I found it very helpful to have these comments. If you come up with a
replacement soon-ish, it's fine to land this as is, otherwise I'd favor
reverting this part of the change for now.

[Sven Panne, 2013/04/22 09:34:25]

As discussed offline, this can and should be handled differently: Sometimes we
need this info, sometimes we need to see the values of some flags, etc. Perhaps
we should just emit what --trace-hydrogen emits. I'll remove the stuff for now,
having the cruel hack here really hurts readability.

-                    current_instruction_, instr->Mnemonic(),
-                    changed_value->id(), changed_value->Mnemonic(),
-                    use_id, use_mnemo);
-          } else {
-            Comment(";;; @%d: %s. <#%d>", current_instruction_,
-                    instr->Mnemonic(), hydrogen->id());
-          }
-        } else {
-          Comment(";;; @%d: %s.", current_instruction_, instr->Mnemonic());
-        }
-      }
-
-      if (!CpuFeatures::IsSupported(SSE2)) {
-        FlushX87StackIfNecessary(instr);
-      }
+    if (FLAG_code_comments && instr->HasInterestingComment(this)) {
+      Comment(";;; <%d,%d> %s",

[Jakob Kummerow, 2013/04/22 09:19:59]

For grep-ability, I like having a '#' before the hydrogen ID. By the same
reasoning you may want to keep the '@' before the Lithium ID (personally I don't
care much about those), so we'd have ";;; <@%d, #%d> %s". 

[Sven Panne, 2013/04/22 09:34:25]

Done.

+              current_instruction_,
+              instr->hydrogen_value()->id(),
+              instr->Mnemonic());
+    }
 
-      instr->CompileToNative(this);
+    if (!CpuFeatures::IsSupported(SSE2)) FlushX87StackIfNecessary(instr);
 
-      if (!CpuFeatures::IsSupported(SSE2)) {
-        ASSERT(!instr->HasDoubleRegisterResult() || x87_stack_depth_ == 1);
+    instr->CompileToNative(this);
 
-        if (FLAG_debug_code && FLAG_enable_slow_asserts) {
-          __ VerifyX87StackDepth(x87_stack_depth_);
-        }
+    if (!CpuFeatures::IsSupported(SSE2)) {
+      ASSERT(!instr->HasDoubleRegisterResult() || x87_stack_depth_ == 1);
+      if (FLAG_debug_code && FLAG_enable_slow_asserts) {
+        __ VerifyX87StackDepth(x87_stack_depth_);
       }
     }
   }
@@ -390,6 +369,9 @@ bool LCodeGen::GenerateBody() {
 bool LCodeGen::GenerateJumpTable() {
   Label needs_frame_not_call;
   Label needs_frame_is_call;
+  if (jump_table_.length() > 0) {
+    Comment(";;; -------------------- Jump table --------------------");
+  }
   for (int i = 0; i < jump_table_.length(); i++) {
     __ bind(&jump_table_[i].label);
     Address entry = jump_table_[i].address;
@@ -465,11 +447,14 @@ bool LCodeGen::GenerateDeferredCode() {
   if (deferred_.length() > 0) {
     for (int i = 0; !is_aborted() && i < deferred_.length(); i++) {
       LDeferredCode* code = deferred_[i];
+      Comment(";;; <%d,%d> "
+              "-------------------- Deferred %s --------------------",
+              code->instruction_index(),
+              code->instr()->hydrogen_value()->id(),
+              code->instr()->Mnemonic());
       __ bind(code->entry());
       if (NeedsDeferredFrame()) {
-        Comment(";;; Deferred build frame @%d: %s.",
-                code->instruction_index(),
-                code->instr()->Mnemonic());
+        Comment(";;; Build frame");
         ASSERT(!frame_is_built_);
         ASSERT(info()->IsStub());
         frame_is_built_ = true;
@@ -478,15 +463,11 @@ bool LCodeGen::GenerateDeferredCode() {
         __ push(Operand(ebp, StandardFrameConstants::kContextOffset));
         __ push(Immediate(Smi::FromInt(StackFrame::STUB)));
         __ lea(ebp, Operand(esp, 2 * kPointerSize));
+        Comment(";;; Deferred code");
       }
-      Comment(";;; Deferred code @%d: %s.",
-              code->instruction_index(),
-              code->instr()->Mnemonic());
       code->Generate();
       if (NeedsDeferredFrame()) {
-        Comment(";;; Deferred destroy frame @%d: %s.",
-                code->instruction_index(),
-                code->instr()->Mnemonic());
+        Comment(";;; Destroy frame");
         ASSERT(frame_is_built_);
         frame_is_built_ = false;
         __ mov(esp, ebp);
@@ -1125,10 +1106,19 @@ void LCodeGen::RecordPosition(int position) {
 }
 
 
+static const char* LabelType(LLabel* label) {
+  if (label->is_loop_header()) return " (loop header)";
+  if (label->is_osr_entry()) return " (OSR entry)";
+  return "";
+}
+
+
 void LCodeGen::DoLabel(LLabel* label) {
-  Comment(";;; -------------------- B%d%s --------------------",
+  Comment(";;; <%d,%d> -------------------- B%d%s --------------------",
+          current_instruction_,
+          label->hydrogen_value()->id(),
           label->block_id(),
-          label->is_loop_header() ? " (loop header)" : "");
+          LabelType(label));
   __ bind(label->label());
   current_block_ = label->block_id();
   DoGap(label);
@@ -2056,7 +2046,7 @@ void LCodeGen::DoArithmeticT(LArithmeticT* instr) {
 }
 
 
-int LCodeGen::GetNextEmittedBlock() {
+int LCodeGen::GetNextEmittedBlock() const {
   for (int i = current_block_ + 1; i < graph()->blocks()->length(); ++i) {
     if (!chunk_->GetLabel(i)->HasReplacement()) return i;
   }