[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

Alisamar Husain via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Mar 31 00:31:31 PDT 2022

zrthxn added inline comments.

Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:45-52
+    if (!m_current_tsc)
+      m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+    else if (!m_current_tsc->InRange(m_pos)) {
+      if (m_pos > m_current_tsc->GetEnd())
+        m_current_tsc = m_current_tsc->Next();
+      if (m_pos < m_current_tsc->GetStart())
+        m_current_tsc = m_current_tsc->Prev();
wallace wrote:
> No need to do `m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);` because its value has already been calculated in the constructor. We can simplify this as well
It is possible that when TraceCursorIntelPT is created the m_current_tsc is None, for example when just started the trace and tried to dump instructions... But then if a tsc is emitted later, this would cause it to remain None since we don't re-calculate it if it was initially None

Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:102-103
-Optional<uint64_t> TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+  if (!m_current_tsc)
wallace wrote:
> are you using git clang-format? I'm curious why this line changed
Yes  I am. I think its because its longer than 80 chars.

Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:104-110
+  if (!m_current_tsc)
+    return None;
   switch (counter_type) {
-    case lldb::eTraceCounterTSC:
-      return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+  case lldb::eTraceCounterTSC:
+    return m_current_tsc->GetTsc();
wallace wrote:
m_current_tsc is already checked at the beginning of this function

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list