8204267: Generate comments in -XX:+PrintInterpreter to link to source code
authoriklam
Sun, 03 Jun 2018 23:33:00 -0700
changeset 51056 3ddf41505d54
parent 51055 a3183ce753e6
child 51057 13ed0d538b89
8204267: Generate comments in -XX:+PrintInterpreter to link to source code Summary: Changed __ macro to use Disassembler::hook() Reviewed-by: coleenp, aph
src/hotspot/cpu/x86/methodHandles_x86.cpp
src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp
src/hotspot/cpu/x86/templateInterpreterGenerator_x86_32.cpp
src/hotspot/cpu/x86/templateInterpreterGenerator_x86_64.cpp
src/hotspot/cpu/x86/templateTable_x86.cpp
src/hotspot/share/compiler/disassembler.cpp
src/hotspot/share/compiler/disassembler.hpp
src/hotspot/share/interpreter/templateInterpreterGenerator.cpp
--- a/src/hotspot/cpu/x86/methodHandles_x86.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/cpu/x86/methodHandles_x86.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "jvm.h"
 #include "asm/macroAssembler.hpp"
+#include "compiler/disassembler.hpp"
 #include "classfile/javaClasses.inline.hpp"
 #include "interpreter/interpreter.hpp"
 #include "interpreter/interpreterRuntime.hpp"
@@ -35,7 +36,7 @@
 #include "runtime/frame.inline.hpp"
 #include "utilities/preserveException.hpp"
 
-#define __ _masm->
+#define __ Disassembler::hook<MacroAssembler>(__FILE__, __LINE__, _masm)->
 
 #ifdef PRODUCT
 #define BLOCK_COMMENT(str) /* nothing */
--- a/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include "asm/macroAssembler.hpp"
+#include "compiler/disassembler.hpp"
 #include "gc/shared/barrierSetAssembler.hpp"
 #include "interpreter/bytecodeHistogram.hpp"
 #include "interpreter/interp_masm.hpp"
@@ -48,7 +49,7 @@
 #include "utilities/debug.hpp"
 #include "utilities/macros.hpp"
 
-#define __ _masm->
+#define __ Disassembler::hook<InterpreterMacroAssembler>(__FILE__, __LINE__, _masm)->
 
 // Size of interpreter code.  Increase if too small.  Interpreter will
 // fail with a guarantee ("not enough space for interpreter generation");
@@ -1774,18 +1775,30 @@
                                                          address& vep) {
   assert(t->is_valid() && t->tos_in() == vtos, "illegal template");
   Label L;
-  aep = __ pc();  __ push_ptr();   __ jmp(L);
+  aep = __ pc();     // atos entry point
+      __ push_ptr();
+      __ jmp(L);
 #ifndef _LP64
-  fep = __ pc(); __ push(ftos); __ jmp(L);
-  dep = __ pc(); __ push(dtos); __ jmp(L);
+  fep = __ pc();     // ftos entry point
+      __ push(ftos);
+      __ jmp(L);
+  dep = __ pc();     // dtos entry point
+      __ push(dtos);
+      __ jmp(L);
 #else
-  fep = __ pc();  __ push_f(xmm0); __ jmp(L);
-  dep = __ pc();  __ push_d(xmm0); __ jmp(L);
+  fep = __ pc();     // ftos entry point
+      __ push_f(xmm0);
+      __ jmp(L);
+  dep = __ pc();     // dtos entry point
+      __ push_d(xmm0);
+      __ jmp(L);
 #endif // _LP64
-  lep = __ pc();  __ push_l();     __ jmp(L);
-  bep = cep = sep =
-  iep = __ pc();  __ push_i();
-  vep = __ pc();
+  lep = __ pc();     // ltos entry point
+      __ push_l();
+      __ jmp(L);
+  bep = cep = sep = iep = __ pc();      // [bcsi]tos entry point
+      __ push_i();
+  vep = __ pc();    // vtos entry point
   __ bind(L);
   generate_and_dispatch(t);
 }
--- a/src/hotspot/cpu/x86/templateInterpreterGenerator_x86_32.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/cpu/x86/templateInterpreterGenerator_x86_32.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include "asm/macroAssembler.hpp"
+#include "compiler/disassembler.hpp"
 #include "interpreter/interp_masm.hpp"
 #include "interpreter/interpreter.hpp"
 #include "interpreter/interpreterRuntime.hpp"
@@ -31,7 +32,7 @@
 #include "runtime/arguments.hpp"
 #include "runtime/sharedRuntime.hpp"
 
-#define __ _masm->
+#define __ Disassembler::hook<InterpreterMacroAssembler>(__FILE__, __LINE__, _masm)->
 
 
 address TemplateInterpreterGenerator::generate_slow_signature_handler() {
--- a/src/hotspot/cpu/x86/templateInterpreterGenerator_x86_64.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/cpu/x86/templateInterpreterGenerator_x86_64.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include "asm/macroAssembler.hpp"
+#include "compiler/disassembler.hpp"
 #include "interpreter/interp_masm.hpp"
 #include "interpreter/interpreter.hpp"
 #include "interpreter/interpreterRuntime.hpp"
@@ -31,7 +32,7 @@
 #include "runtime/arguments.hpp"
 #include "runtime/sharedRuntime.hpp"
 
-#define __ _masm->
+#define __ Disassembler::hook<InterpreterMacroAssembler>(__FILE__, __LINE__, _masm)->
 
 #ifdef _WIN64
 address TemplateInterpreterGenerator::generate_slow_signature_handler() {
--- a/src/hotspot/cpu/x86/templateTable_x86.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/cpu/x86/templateTable_x86.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include "asm/macroAssembler.hpp"
+#include "compiler/disassembler.hpp"
 #include "interpreter/interpreter.hpp"
 #include "interpreter/interpreterRuntime.hpp"
 #include "interpreter/interp_masm.hpp"
@@ -40,7 +41,7 @@
 #include "runtime/synchronizer.hpp"
 #include "utilities/macros.hpp"
 
-#define __ _masm->
+#define __ Disassembler::hook<InterpreterMacroAssembler>(__FILE__, __LINE__, _masm)->
 
 // Global Register Names
 static const Register rbcp     = LP64_ONLY(r13) NOT_LP64(rsi);
--- a/src/hotspot/share/compiler/disassembler.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/share/compiler/disassembler.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "asm/macroAssembler.hpp"
 #include "ci/ciUtilities.hpp"
 #include "classfile/javaClasses.hpp"
 #include "code/codeCache.hpp"
@@ -36,6 +37,7 @@
 #include "runtime/os.hpp"
 #include "runtime/stubCodeGenerator.hpp"
 #include "runtime/stubRoutines.hpp"
+#include "utilities/resourceHash.hpp"
 #include CPU_HEADER(depChecker)
 
 void*       Disassembler::_library               = NULL;
@@ -49,7 +51,7 @@
 static const char decode_instructions_virtual_name[] = "decode_instructions_virtual";
 static const char decode_instructions_name[] = "decode_instructions";
 static bool use_new_version = true;
-#define COMMENT_COLUMN  40 LP64_ONLY(+8) /*could be an option*/
+#define COMMENT_COLUMN  52 LP64_ONLY(+8) /*could be an option*/
 #define BYTES_COMMENT   ";..."  /* funky byte display comment */
 
 bool Disassembler::load_library() {
@@ -163,6 +165,7 @@
   bool          _print_bytes;
   address       _cur_insn;
   int           _bytes_per_line; // arch-specific formatting option
+  bool          _print_file_name;
 
   static bool match(const char* event, const char* tag) {
     size_t taglen = strlen(tag);
@@ -191,6 +194,51 @@
   void print_insn_bytes(address pc0, address pc);
   void print_address(address value);
 
+  struct SourceFileInfo {
+    struct Link : public CHeapObj<mtCode> {
+      const char* file;
+      int line;
+      Link* next;
+      Link(const char* f, int l) : file(f), line(l), next(NULL) {}
+    };
+    Link *head, *tail;
+
+    static unsigned hash(const address& a) {
+      return primitive_hash<address>(a);
+    }
+    static bool equals(const address& a0, const address& a1) {
+      return primitive_equals<address>(a0, a1);
+    }
+    void append(const char* file, int line) {
+      if (tail != NULL && tail->file == file && tail->line == line) {
+        // Don't print duplicated lines at the same address. This could happen with C
+        // macros that end up having multiple "__" tokens on the same __LINE__.
+        return;
+      }
+      Link *link = new Link(file, line);
+      if (head == NULL) {
+        head = tail = link;
+      } else {
+        tail->next = link;
+        tail = link;
+      }
+    }
+    SourceFileInfo(const char* file, int line) : head(NULL), tail(NULL) {
+      append(file, line);
+    }
+  };
+
+  typedef ResourceHashtable<
+      address, SourceFileInfo,
+      SourceFileInfo::hash,
+      SourceFileInfo::equals,
+      15889,      // prime number
+      ResourceObj::C_HEAP> SourceFileInfoTable;
+
+  static SourceFileInfoTable _src_table;
+  static const char* _cached_src;
+  static GrowableArray<const char*>* _cached_src_lines;
+
  public:
   decode_env(CodeBlob* code, outputStream* output,
              CodeStrings c = CodeStrings(), ptrdiff_t offset = 0);
@@ -212,6 +260,7 @@
       _nm->print_code_comment_on(st, COMMENT_COLUMN, pc0, pc);
       // this calls reloc_string_for which calls oop::print_value_on
     }
+    print_hook_comments(pc0, _nm != NULL);
     // follow each complete insn by a nice newline
     st->cr();
   }
@@ -221,8 +270,96 @@
   outputStream* output() { return _output; }
   address cur_insn() { return _cur_insn; }
   const char* options() { return _option_buf; }
+  static void hook(const char* file, int line, address pc);
+  void print_hook_comments(address pc, bool newline);
 };
 
+decode_env::SourceFileInfoTable decode_env::_src_table;
+const char* decode_env::_cached_src = NULL;
+GrowableArray<const char*>* decode_env::_cached_src_lines = NULL;
+
+void decode_env::hook(const char* file, int line, address pc) {
+  // For simplication, we never free from this table. It's really not
+  // necessary as we add to the table only when PrintInterpreter is true,
+  // which means we are debugging the VM and a little bit of extra
+  // memory usage doesn't matter.
+  SourceFileInfo* found = _src_table.get(pc);
+  if (found != NULL) {
+    found->append(file, line);
+  } else {
+    SourceFileInfo sfi(file, line);
+    _src_table.put(pc, sfi); // sfi is copied by value
+  }
+}
+
+void decode_env::print_hook_comments(address pc, bool newline) {
+  SourceFileInfo* found = _src_table.get(pc);
+  outputStream* st = output();
+  if (found != NULL) {
+    for (SourceFileInfo::Link *link = found->head; link; link = link->next) {
+      const char* file = link->file;
+      int line = link->line;
+      if (_cached_src == NULL || strcmp(_cached_src, file) != 0) {
+        FILE* fp;
+
+        // _cached_src_lines is a single cache of the lines of a source file, and we refill this cache
+        // every time we need to print a line from a different source file. It's not the fastest,
+        // but seems bearable.
+        if (_cached_src_lines != NULL) {
+          for (int i=0; i<_cached_src_lines->length(); i++) {
+            os::free((void*)_cached_src_lines->at(i));
+          }
+          _cached_src_lines->clear();
+        } else {
+          _cached_src_lines = new (ResourceObj::C_HEAP, mtCode)GrowableArray<const char*>(0, true);
+        }
+
+        if ((fp = fopen(file, "r")) == NULL) {
+          _cached_src = NULL;
+          return;
+        }
+        _cached_src = file;
+
+        char line[500]; // don't write lines that are too long in your source files!
+        while (fgets(line, sizeof(line), fp) != NULL) {
+          size_t len = strlen(line);
+          if (len > 0 && line[len-1] == '\n') {
+            line[len-1] = '\0';
+          }
+          _cached_src_lines->append(os::strdup(line));
+        }
+        fclose(fp);
+        _print_file_name = true;
+      }
+
+      if (_print_file_name) {
+        // We print the file name whenever we switch to a new file, or when
+        // Disassembler::decode is called to disassemble a new block of code.
+        _print_file_name = false;
+        if (newline) {
+          st->cr();
+        }
+        st->move_to(COMMENT_COLUMN);
+        st->print(";;@FILE: %s", file);
+        newline = true;
+      }
+
+      int index = line - 1; // 1-based line number -> 0-based index.
+      if (index >= _cached_src_lines->length()) {
+        // This could happen if source file is mismatched.
+      } else {
+        const char* source_line = _cached_src_lines->at(index);
+        if (newline) {
+          st->cr();
+        }
+        st->move_to(COMMENT_COLUMN);
+        st->print(";;%5d: %s", line, source_line);
+        newline = true;
+      }
+    }
+  }
+}
+
 decode_env::decode_env(CodeBlob* code, outputStream* output, CodeStrings c,
                        ptrdiff_t offset) {
   memset(this, 0, sizeof(*this)); // Beware, this zeroes bits of fields.
@@ -237,6 +374,7 @@
   _print_pc       = true;
   _print_bytes    = false;
   _bytes_per_line = Disassembler::pd_instruction_alignment();
+  _print_file_name= true;
 
   // parse the global option string:
   collect_options(Disassembler::pd_cpu_opts());
@@ -558,3 +696,9 @@
 
   env.decode_instructions(p, end);
 }
+
+// To prevent excessive code expansion in the interpreter generator, we
+// do not inline this function into Disassembler::hook().
+void Disassembler::_hook(const char* file, int line, MacroAssembler* masm) {
+  decode_env::hook(file, line, masm->code_section()->end());
+}
--- a/src/hotspot/share/compiler/disassembler.hpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/share/compiler/disassembler.hpp	Sun Jun 03 23:33:00 2018 -0700
@@ -77,6 +77,17 @@
   static void decode(nmethod* nm,                outputStream* st = NULL);
   static void decode(address begin, address end, outputStream* st = NULL,
                      CodeStrings c = CodeStrings(), ptrdiff_t offset = 0);
+  static void _hook(const char* file, int line, class MacroAssembler* masm);
+
+  // This functions makes it easy to generate comments in the generated
+  // interpreter code, by riding on the customary __ macro in the interpreter generator.
+  // See templateTable_x86.cpp for an example.
+  template<class T> inline static T* hook(const char* file, int line, T* masm) {
+    if (PrintInterpreter) {
+      _hook(file, line, masm);
+    }
+    return masm;
+  }
 };
 
 #endif // SHARE_VM_COMPILER_DISASSEMBLER_HPP
--- a/src/hotspot/share/interpreter/templateInterpreterGenerator.cpp	Fri Jul 13 10:10:51 2018 -0700
+++ b/src/hotspot/share/interpreter/templateInterpreterGenerator.cpp	Sun Jun 03 23:33:00 2018 -0700
@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "compiler/disassembler.hpp"
 #include "interpreter/interpreter.hpp"
 #include "interpreter/interpreterRuntime.hpp"
 #include "interpreter/interp_masm.hpp"
@@ -33,7 +34,7 @@
 
 #ifndef CC_INTERP
 
-# define __ _masm->
+#define __ Disassembler::hook<InterpreterMacroAssembler>(__FILE__, __LINE__, _masm)->
 
 TemplateInterpreterGenerator::TemplateInterpreterGenerator(StubQueue* _code): AbstractInterpreterGenerator(_code) {
   _unimplemented_bytecode    = NULL;