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/DecodedThread.h:172 std::vector<IntelPTInstruction> m_instructions; + std::vector<llvm::Error> m_errors; + ---------------- wallace wrote: > you need to have something like > std::unordered_map<uint64_t, llvm::Error> m_errors; > > that way, you'll be able to quickly look for the error associated with an > instruction index. The IntelPTInstruction int his case, instead of storing > the Error, can just store one bit of information has_error = true/false; nit: from https://llvm.org/docs/CodingStandards.html#c-standard-library prefer `llvm::DenseMap` over `std::map`'s unless there's a specific reason not to. Don't forget to update `CalculateApproximateMemoryUsage()` as well! Also, besides for being inline with the coding standards I linked above, using `llvm::DenseMap` here has the actual advantage that it exposes its approximate size via `getMemorySize()`, whereas there is no easy way to get the size of `std::map`. ================ Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:120 + thread.AppendError(make_error<IntelPTError>(errcode)); + // instructions.emplace_back(make_error<IntelPTError>(errcode)); break; ---------------- Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122293/new/ https://reviews.llvm.org/D122293 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
