[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 requested changes to this revision.
jj10306 added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:164-166
+  /// \param[in] decoder
+  ///     A decoder configured to start and end within the boundaries of the
+  ///     given \p psb_block.
----------------
should this be trace_intel_pt?



================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:476-478
+      if (event.has_tsc) {
+        tsc = event.tsc;
+        break;
----------------
so is this inner loop what's actually doing the work of getting to the next PSB? is the assumption that if an event has a tsc then it's a PSB?
Can you explain what the two different while loops are doing?


================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:109
+llvm::Expected<std::vector<PSBBlock>>
 SplitTraceInContinuousExecutions(TraceIntelPT &trace_intel_pt,
+                                 llvm::ArrayRef<uint8_t> buffer,
----------------
should we rename now that we are using PSBBlock everywhere?


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp:95
-static Expected<std::vector<IntelPTThreadSubtrace>>
+static Expected<std::vector<PSBBlock>>
 GetIntelPTSubtracesForCpu(TraceIntelPT &trace, cpu_id_t cpu_id) {
-  std::vector<IntelPTThreadSubtrace> intel_pt_subtraces;
----------------
rename?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131630