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) {
+Optional<uint64_t>
+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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to