jj10306 added a comment. Overall looks good, just a couple cosmetic things and comments about the plan for multi-buffer, single thread decoding
================ Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:41 + +/// Class that decodes a raw buffer for a single thread using the low level +/// libipt library. ---------------- "for a single thread" thinking ahead for the multi-CPU buffer case - this class will be responsible for decoding a single trace buffer, not necessarily a single thread's buffer, right? Also, come multi-buffer time, the notion of `DecodeThread` in this class will need to be changed to `DecodedBuffer` or something similar that decouples the decoder from a particular thread. No need to change this now but wanted to make sure I'm understanding the plan correctly. ================ Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:103 + /// The libipt decoder status after moving to the next PSB. Negative if + /// not PSB was found. + int FindNextSynchronizationPoint() { ---------------- ================ Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:112 + + if (status != -pte_eos && IsLibiptError(status)) { + uint64_t decoder_offset = 0; ---------------- ================ Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:152 + status = pt_insn_event(&m_decoder, &event, sizeof(event)); + if (status < 0) + break; ---------------- ================ Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:253 +using PtInsnDecoderUP = + std::unique_ptr<pt_insn_decoder, decltype(DecoderDeleter)>; + ---------------- nice custom deleter (: ================ Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:23 +/// library underneath. +void DecodeInMemoryTrace(DecodedThread &decoded_thread, + TraceIntelPT &trace_intel_pt, ---------------- If this file is just a wrapper around Libipt and doesn’t have any context about how LLDB gets a trace (from the aux buffer or a file), should we rename it to ‘DecodeTrace’ and the caller is responsible for providing the trace data in a byte buffer? In the case of in memory trace, the caller already has the byte buffer. In the case of a file, they could mmap the file and cast it to the appropriate type? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123106/new/ https://reviews.llvm.org/D123106 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits