jj10306 requested changes to this revision.
jj10306 added a comment.
This revision now requires changes to proceed.
looks good overall, mainly some questions and a few nits
================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:123
/// of a DenseMap because DenseMap can't understand enums.
- std::unordered_map<lldb::TraceEvent, size_t> events_counts;
- size_t total_count = 0;
+ std::unordered_map<lldb::TraceEvent, uint64_t> events_counts;
+ uint64_t total_count = 0;
----------------
================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:130
+ // Struct holding counts for errors
+ struct ErrorStats {
+ /// The following counters are mutually exclusive
----------------
nice, I was about to add this as part of my diff (:
================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:206
+ }
+ m_next_infinite_decoding_loop_threshold *= 2;
+ }
----------------
can you explain why we are increasing the threshold?
================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:235-241
+ if (m_decoded_thread.GetInstructionLoadAddress(last_insn_index) !=
+ eTraceItemKindInstruction) {
+ if (Optional<uint64_t> prev_index = prev_insn_index(last_insn_index)) {
+ last_insn_index = *prev_index;
+ } else {
+ return None;
+ }
----------------
if you move the `--item_index` in `prev_insn_index` lambda, would that allow
you to remove this duplicated `eTraceItemKindInstruction` check and instead
unconditionally call `prev_insn_index`?
or would this not work because the intention of the lamda is to skip the
current event even if it's already an instruction
================
Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:281
+ lldb::addr_t new_packet_offset;
+ if (!IsLibiptError(pt_insn_get_offset(&m_decoder, &new_packet_offset)) &&
+ new_packet_offset != m_last_packet_offset) {
----------------
help me understand this please. I thought `pt_insn_get_offset` would always
return a new, increasing offset every time this function is called.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136557/new/
https://reviews.llvm.org/D136557
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
