[Lldb-commits] [PATCH] D122293: [intelpt] Refactoring instruction decoding for flexibility

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:25
 IntelPTError::IntelPTError(int libipt_error_code, lldb::addr_t address)
     : m_libipt_error_code(libipt_error_code), m_address(address) {
   assert(libipt_error_code < 0);
----------------



================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:105-107
 DecodedThread::DecodedThread(ThreadSP thread_sp, Error error)
     : m_thread_sp(thread_sp) {
+  m_instructions.emplace_back(error);
----------------
let's improve this method a bit


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:86-87
 
-  /// \return
-  ///     An \a llvm::Error object if this class corresponds to an Error, or an
-  ///     \a llvm::Error::success otherwise.
-  llvm::Error ToError() const;
+  /// Get the size in bytes of an instance of this class
+  static size_t GetMemoryUsage();
 
----------------
good


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:155
+  /// Append a successfully decoded instruction.
+  template <typename... Ts> void AppendInstruction(Ts... __args) {
+    m_instructions.emplace_back(std::move(IntelPTInstruction(__args...)));
----------------
just call it args or instruction_args instead of __args


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:156
+  template <typename... Ts> void AppendInstruction(Ts... __args) {
+    m_instructions.emplace_back(std::move(IntelPTInstruction(__args...)));
+  }
----------------
the goal is to avoid creating explicitly a  IntelPTInstruction, so you should be able to achieve something like    
  m_instructions.emplace_back(args...);



================
Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:205
 
-  int errcode = pt_image_set_callback(image, ReadProcessMemory, &process);
+  // ProcessSP process = thread_sp->GetProcess();
+  int errcode = pt_image_set_callback(image, ReadProcessMemory,
----------------
delete this


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:122-123
            (double)mem_used / 1024);
+  s.Printf("  Average memory usage per instruction: %0.2lf KiB\n",
+           ((double)mem_used/insn_len) / 1024);
   return;
----------------
print it in bytes instead


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122293/new/

https://reviews.llvm.org/D122293