[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 requested changes to this revision.
jj10306 added a comment.
This revision now requires changes to proceed.

Couple minor things



================
Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:66
 private:
+  /// Indicate the dumper that no more data is available in the trace.
+  /// This will prevent further iterations.
----------------



================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:54
 
-size_t TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) {
+int64_t TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) {
   int64_t last_index = GetInternalInstructionSize() - 1;
----------------
Should this return an unsigned int? Currently, it appears the return value will always be >= 0


================
Comment at: lldb/source/Target/TraceInstructionDumper.cpp:33
+  } else if (forwards) {
+    m_cursor_up->Seek(0, TraceCursor::SeekType::Set);
+  } else {
----------------
nit: Given that we have `SeekType::End`, why not rename `SeekType::Set`, to `SeekType::Start`? `SeekType::Set` was a little confusing to me the first time I read it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122254