[Lldb-commits] [PATCH] D105741: [trace] Add `thread trace dump ctf -f <filename>` command
walter erquinigo via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 13 14:20:22 PDT 2021
wallace added inline comments.
================
Comment at: lldb/include/lldb/Target/TraceHTR.h:31
+public:
+ HTRBlockMetadata(Thread &thread, TraceInstruction curr_instruction,
+ llvm::Optional<TraceInstruction> next_instruction);
----------------
const TraceInstruction &curr_instruction
to avoid making an unnecessary copy
================
Comment at: lldb/include/lldb/Target/TraceHTR.h:33
+ llvm::Optional<TraceInstruction> next_instruction);
+ HTRBlockMetadata(size_t num_instructions,
+ std::unordered_map<std::string, size_t> func_calls)
----------------
pass the map by reference
================
Comment at: lldb/include/lldb/Target/TraceHTR.h:42-68
+ friend llvm::json::Value toJSON(const HTRBlockMetadata &metadata);
+ template <class M> friend llvm::json::Value toJSON(const TraceHTR<M> &layer);
+};
+
+/// \class HTRBlock TraceHTR.h "lldb/Target/TraceHTR.h"
+/// Trace agnostic block structure
+/// Sequences of blocks are merged to create a new, single block
----------------
the consistent style is that toJSON methods are not class members but static functions
================
Comment at: lldb/include/lldb/Target/TraceHTR.h:73
+/// Maps the unique Block IDs to their respective Blocks
+template <class TMetadata> class HTRBlockDefs {
+public:
----------------
clayborg wrote:
> Does this need to be templatized? Are we using this same class with multiple different TMetadata classes or are we always using it with HTRBlockMetadata?
+1
I don't think you should go with templates unless you had more TMetadata objects
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
https://reviews.llvm.org/D105741
More information about the lldb-commits
mailing list