wallace accepted this revision.
wallace added a comment.
Nice! I'll land this for you
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
https://reviews.llvm.org/D105741
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
jj10306 updated this revision to Diff 361916.
jj10306 added a comment.
Rebased (integrate with https://reviews.llvm.org/D106501#change-SoRRVpoqDNdx)
and address comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
https://reviews.llvm.org/D105741
Files:
wallace added a comment.
Pretty good. Just some minor nits. Once you rebase on top of the trace exporter
plugin patch, this should be good to go.
Comment at: lldb/test/API/commands/trace/TestTraceExport.py:54-56
+TODO: Once the "trace save" command is implemented,
jj10306 added a comment.
Looks like I pulled in some unwanted changes related to the traceinstruction
dumper and tsc when I rebased - will resolve this issue before landing
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
https://reviews.llvm.org/D105741
jj10306 updated this revision to Diff 361460.
jj10306 edited the summary of this revision.
jj10306 added a comment.
Rebase and address comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
https://reviews.llvm.org/D105741
Files:
lldb/docs/htr.rst
wallace added inline comments.
Comment at: lldb/source/Target/TraceHTR.cpp:160
+ current_instruction_load_address);
+ valid_cursor = cursor.Next();
+ if (current_instruction_type &
jj10306 wrote:
> wallace wrote:
> > valid_cursor is not a nice
jj10306 marked 29 inline comments as done.
jj10306 added inline comments.
Comment at: lldb/include/lldb/Target/TraceHTR.h:272
+ /// The map of block IDs to \a HTRBlock.
+ std::unordered_map const () const;
+
jj10306 wrote:
> wallace wrote:
> > jj10306
jj10306 marked an inline comment as done.
jj10306 added inline comments.
Comment at: lldb/include/lldb/Target/TraceHTR.h:272
+ /// The map of block IDs to \a HTRBlock.
+ std::unordered_map const () const;
+
wallace wrote:
> jj10306 wrote:
> > wallace
wallace added inline comments.
Comment at: lldb/include/lldb/Target/TraceHTR.h:272
+ /// The map of block IDs to \a HTRBlock.
+ std::unordered_map const () const;
+
jj10306 wrote:
> wallace wrote:
> > As this is const, the only way to use this
jj10306 marked 2 inline comments as done.
jj10306 added inline comments.
Comment at: lldb/include/lldb/Target/TraceHTR.h:272
+ /// The map of block IDs to \a HTRBlock.
+ std::unordered_map const () const;
+
wallace wrote:
> As this is const, the only way
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
So much better. There's an optimization that you can make when merging maps,
and besides that you need a more comprehensive test.
Comment at:
jj10306 updated this revision to Diff 360965.
jj10306 marked 2 inline comments as done.
jj10306 added a comment.
Address comments:
- use unique_ptr to prevent unnecessary copying
- add support for trace decoding errors
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
jj10306 marked 39 inline comments as done.
jj10306 added inline comments.
Comment at: lldb/source/Target/TraceHTR.cpp:53-57
+std::vector const ::GetBlockLayers() const {
+ return m_block_layers;
+}
+
+HTRInstructionLayer const ::GetInstructionLayer() const {
wallace added inline comments.
Comment at: lldb/include/lldb/Target/TraceHTR.h:136-140
+ /// Updates block_id: block mapping and append `block_id` to the block id
+ /// trace
+ void AddNewBlock(size_t block_id, HTRBlock );
+ /// Append `block_id` to the block id trace
+
jj10306 added inline comments.
Comment at: lldb/include/lldb/Target/TraceHTR.h:136-140
+ /// Updates block_id: block mapping and append `block_id` to the block id
+ /// trace
+ void AddNewBlock(size_t block_id, HTRBlock );
+ /// Append `block_id` to the block id trace
+
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
Much better! We are getting closer. There's documentation missing. Besides, you
should use unique_ptrs as mentioned in the comments so that you prevent copying
massively large
jj10306 added inline comments.
Comment at: lldb/source/Target/TraceHTR.cpp:205-217
+ // Why does using `IHTRLayer _layer = m_instruction_layer;` not work?
+ HTRInstructionLayer instruction_layer = m_instruction_layer;
+ HTRBlockLayer current_layer =
jj10306 marked 19 inline comments as done.
jj10306 added inline comments.
Comment at: lldb/docs/htr.rst:1
+Hierarchical Trace Representation (HTR)
+==
@clayborg @wallace Is there a way to view the "rendered" rst to ensure the
jj10306 updated this revision to Diff 359848.
jj10306 retitled this revision from "[trace] Add `thread trace dump ctf -f
` command" to "[trace] Add `thread trace export` command for Intel PT
trace visualization".
jj10306 edited the summary of this revision.
jj10306 added a comment.
- Add HTR
19 matches
Mail list logo
